C wrapper: connect2/disconnect2 are no-op stubs that return 200, then EnableTwin hangs forever

[ewertons]

Summary

The C wrapper's connect2 and disconnect2 REST handlers are unmodified swagger-generated stubs. They return 200 OK without doing anything, so a caller believes the client reconnected when it did not. A subsequent enableTwin then blocks forever on an unbounded std::condition_variable::wait, wedging the wrapper until the test client's 150s read timeout fires.

This makes test_twin_desired_props_patch fail intermittently in the c_*_edgehub_module suites, e.g. horton-gate-build 161937.

Detail

test-runner/twin_tests.py has a recovery path for a desired-property patch that does not arrive in the first wait window: disconnect2() -> connect2_with_retry() -> enable_twin(), to force EdgeHub to rebuild its cloud proxy.

Both handlers are pure stubs:

docker_images/c/wrapper/generated/ModuleApi.cpp:193 (connect2) and :532 (disconnect2):

    // Change the value of this variable to the appropriate response before sending the response
    int status_code = 200;

    /**
     * Process the received information here
     */

    if (status_code == 200) {
        session->close(200, "", { {"Connection", "close"} });
        return;
    }

There is no corresponding Connect2/Disconnect2 in glue/InternalGlue.h, InternalGlue.cpp or ModuleGlue.cpp at all.

Then InternalGlue::EnableTwin waits with no timeout — glue/InternalGlue.cpp:212-216:

    std::cout << "waiting for initial Twin response" << std::endl;
    {
        std::unique_lock<std::mutex> lk(resp->m);
        resp->cv.wait(lk, [resp]{ return !resp->latest_payload.empty(); });
    }

Because the client was never actually reopened, no MQTT traffic is generated and latest_payload is never populated, so this never returns.

Evidence from build 161937

testMod.log for test_linux_amd64_c_mqttws_edgehub_module. Note that the disconnect2 and connect2 routes are received but produce no InternalGlue:: log line, unlike every other route:

{"message": "PYTEST: patch 3 not received after 45s"}
{"message": "PYTEST: patch 3 not received, reconnecting to rebuild EdgeHub cloud proxy"}
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/disconnect2'.
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/connect2'.
RESTBED:INFO: Incoming 'PUT' request ... for route '/module/moduleClient_28/enableTwin'.
InternalGlue::EnableTwin for moduleClient_28
waiting for initial Twin response
{"message": "PYTEST: TEST FAILED BACAUSE OF ... Read timed out. (read timeout=150)"}

For contrast, the first enableTwin on the same connection logs a full CONNECT/CONNACK/SUBSCRIBE/\/twin/GET exchange and returns immediately. After the fake reconnect there is no MQTT traffic at all.

The wrapper stays wedged afterwards, so the following test also fails on setup:

failed on setup with "... Max retries exceeded with url: /module/connectFromEnvironment/mqttws
 (Caused by ReadTimeoutError(... read timeout=150))"

Suggested fix

1. Implement Connect2/Disconnect2 in the C glue and call them from the two generated handlers. Semantics, per the node and python wrappers: disconnect2 closes the client but keeps it in clientMap under the same connectionId; connect2 reopens that same client. Note a pending WaitForDesiredPropertyPatch must survive the cycle.
2. Bound the wait in EnableTwin (and the other cv.wait call sites) with wait_for, throwing on expiry. Even once (1) is fixed, an unbounded wait turns any lost twin response into a wedged wrapper and an opaque client-side timeout instead of a clear error.

Azure/azure-iot-sdk-java#1859 was the same class of bug in the Java SDK, and #436 fixes the equivalent gap in the Java wrapper, where moduleConnect2/moduleDisconnect2 threw UnsupportedOperationException. The C wrapper is arguably worse because it reports success rather than failing loudly.