Abort() while HAPAccessoryServerStop

[CrowdedFuzzball]

Hello!

In some cases, stopping the accessory server can be helpful.
For example, if you have Bridge.
To update the list of devices working through the bridge, Apple recommends using the HAPAccessoryServerStop function and then restarting through the HAPAccessoryServerStartBridge.

Unfortunately, in this fork, when trying to stop the server, the following error occurs:

After invoke HAPAccessoryServerStop(BridgeAccessoryConfiguration.server) inside scheduled callback

Fault	assertion failed - HAPPlatformLogPOSIXError @ ../esp-apple-homekit-adk/port/src/HAPPlatformLog.c:65
abort() was called at PC 0x400d673f on core 0
0x400d673f: _exit at ../esp-idf/components/newlib/syscalls.c:67

0x40090188: invoke_abort at ../esp-idf/components/esp32/panic.c:159
0x40090539: abort at ../esp-idf/components/esp32/panic.c:174
0x400d673f: _exit at ../esp-idf/components/newlib/syscalls.c:67
0x401d452a: exit at /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/xtensa-esp32-elf/src/newlib/newlib/libc/stdlib/exit.c:64
0x4010e049: HAPPlatformAbort at ../esp-apple-homekit-adk/port/src/HAPPlatformAbort.c:27
0x40100454: HAPAssertInternal at ../esp-apple-homekit-adk/homekit_adk/PAL/HAPAssert.c:20
0x4010e286: HAPPlatformLogPOSIXError at ../esp-apple-homekit-adk/port/src/HAPPlatformLog.c:65 (discriminator 2)
0x400ffde5: HAPPlatformTCPStreamManagerCloseListener at ../esp-apple-homekit-adk/port/src/HAPPlatformTCPStreamManager.c:332
0x40106f5c: schedule_max_idle_time_timer at ../esp-apple-homekit-adk/homekit_adk/HAP/HAPIPAccessoryServer.c:389
0x40108e0a: handle_server_state_transition_timer at ../esp-apple-homekit-adk/homekit_adk/HAP/HAPIPAccessoryServer.c:3874
0x400ff601: ProcessExpiredTimers at ../esp-apple-homekit-adk/port/src/HAPPlatformRunLoop.c:381
 (inlined by) HAPPlatformRunLoopRun at ../esp-apple-homekit-adk/port/src/HAPPlatformRunLoop.c:638
0x40084d20: HomeKit::Task(void*) at /src/HomeKit/HomeKit.cpp:271

Error occurs here:

    HAPLogDebug(&logObject, "shutdown(%d, SHUT_RDWR);", tcpStreamManager->tcpStreamListener.fileDescriptor);
    e = shutdown(tcpStreamManager->tcpStreamListener.fileDescriptor, SHUT_RDWR);
    if (e != 0) {
        int _errno = errno;
        HAPAssert(e == -1);
        HAPPlatformLogPOSIXError(
                kHAPLogType_Debug,
                "System call 'shutdown' on TCP stream listener socket failed.",
                _errno,
                __func__,
                HAP_FILE,
                __LINE__);
    }

If commented this lines - everything is ok except of memory leaks and i think it is not a good behaviour.

Could you please inspect this issue?

[baysinger]

I'm not sure about the problem in the code you quoted, but I think there's a bug in HAPPlatformLogPOSIXError that causes the assert to fail. The POSIX version of strerror_r returns a string, but the code calling it is written for the XSI version, which return an error code (int).

[baysinger]

Looking a little more carefully at HAPPlatformLog.c, I see there is a compiler guard in there to prevent compiling if the wrong strerror_r is used. (Who thought it would be a good idea to change the signature of stderror_r, anyway?) So I'm not exactly sure what's going on. But it definitely looks like strerror_r is returning a pointer, not an int, at least for me. This means it always fails the assert on line 67: HAPAssert(e == ERANGE);

But you're seeing an assert on line 65, and if you're using the same version of code as me, that means your strerror_r is returning EINVAL. So the that means on your system, one problem is HAPStringWithFormat is returning an error, and another problem is that the error number passed to HAPPlatformLogPOSIXError is not valid.

Either way, the asserts should not fail, and you should not see an abort. But this is sort of a side discussion. The primary problem you're seeing is that shutdown returns an error, and I don't have an answer for that.

[CrowdedFuzzball]

@

Looking a little more carefully at HAPPlatformLog.c, I see there is a compiler guard in there to prevent compiling if the wrong strerror_r is used. (Who thought it would be a good idea to change the signature of stderror_r, anyway?) So I'm not exactly sure what's going on. But it definitely looks like strerror_r is returning a pointer, not an int, at least for me. This means it always fails the assert on line 67: HAPAssert(e == ERANGE);

But you're seeing an assert on line 65, and if you're using the same version of code as me, that means your strerror_r is returning EINVAL. So the that means on your system, one problem is HAPStringWithFormat is returning an error, and another problem is that the error number passed to HAPPlatformLogPOSIXError is not valid.

Either way, the asserts should not fail, and you should not see an abort. But this is sort of a side discussion. The primary problem you're seeing is that shutdown returns an error, and I don't have an answer for that.

Got it, thanks for the clarification.
For our part, we will try to examine the problem more closely.
If we will find solution - report about it.

I don't know if it helps or not, but the compilation takes place on OSX, it is likely that HAPPlatformLogPOSIXError gives an incorrect value in this particular operating system.

[izmmisha]

Got same problem with my code.

Well, HAPPlatformLog.c contains this code:

#ifdef __linux__
#if !defined(_POSIX_C_SOURCE) || _POSIX_C_SOURCE < 200112L
#error "This file needs the XSI-compliant version of 'strerror_r'. Set _POSIX_C_SOURCE >= 200112L."
#endif
#if defined(_GNU_SOURCE) && _GNU_SOURCE
#error "This file needs the XSI-compliant version of 'strerror_r'. Do not set _GNU_SOURCE."
#endif
#endif

as @baysinger mentioned before we already have guards to prevent such problem.
But this guards will work only if __linux__ defined.

so commenting out #ifdef __linux__ and corresponding #endif original guards about wrong strerror_r works, compilation fails with such error:

/home/im/esp-apple-homekit-adk/port/src/HAPPlatformLog.c:39:2: error: #error "This file needs the XSI-compliant version of 'strerror_r'. Do not set _GNU_SOURCE."
 #error "This file needs the XSI-compliant version of 'strerror_r'. Do not set _GNU_SOURCE."
  ^~~~~
/home/im/esp-apple-homekit-adk/port/src/HAPPlatformLog.c: In function 'HAPPlatformLogPOSIXError':
/home/im/esp-apple-homekit-adk/port/src/HAPPlatformLog.c:60:13: warning: initialization of 'int' from 'char *' makes integer from pointer without a cast [-Wint-conversion]
     int e = strerror_r(errorNumber, errorString, sizeof errorString);
             ^~~~~~~~~~

Anyway I looked into assembler of strerror_r in resulting elf:

40166564 <strerror_r>:
40166564:       004136          entry   a1, 32
40166567:       d76c81          l32r    a8, 4015c318 <ram_tx_pwctrl_bg_init+0x288>	4008a598 <__getreent>: 
4016656a:       0008e0          callx8  a8
4016656d:       20b220          or      a11, a2, a2
40166570:       00a0d2          movi    a13, 0
40166573:       01a0c2          movi    a12, 1
40166576:       0a8a25          call8   40170e18 <_strerror_r>
40166579:       0a2d            mov.n   a2, a10
4016657b:       878c81          l32r    a8, 401483ac <cnx_obss_scan_timeout+0x94>	400014c0
4016657e:       0008e0          callx8  a8
40166581:       0bba47          bgeu    a10, a4, 40166590 <strerror_r+0x2c>
40166584:       02bd            mov.n   a11, a2
40166586:       03ad            mov.n   a10, a3
40166588:       d78681          l32r    a8, 4015c3a0 <ram_tx_pwctrl_bg_init+0x310>	400013ac
4016658b:       0008e0          callx8  a8
4016658e:       0a2d            mov.n   a2, a10
40166590:       f01d            retw.n

it looks exactly like this version https://github.com/espressif/newlib-esp32/blob/422607527f8d7543701e83812950153ba2a8e9c0/newlib/libc/string/strerror_r.c#L68

[skanico]

Thanks @izmmisha, your pull request seems to work for me. No more crashes!