C wrapper: connect2/disconnect2 are no-op stubs that return 200, then EnableTwin hangs forever
- Dominant language
- Python
- Stars
- 6
- Forks
- 19
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 10
Description
## 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](https://dev.azure.com/azure-iot-sdks/azure-iot-sdks/_build/results?buildId=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`):
```cpp
// 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`:
```cpp
std::cout << "waiting for initial Twin response" << std::endl;
{
std::unique_lock 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 Azure/iot-sdks-e2e-fx#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.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with docker_images/c/wrapper/generated/ModuleApi.cpp at connect2 (line 193) and disconnect2 (line 532), then compare glue/InternalGlue.h, glue/InternalGlue.cpp, and glue/ModuleGlue.cpp with the node and Python wrapper semantics described. Run test-runner/twin_tests.py, including test_twin_desired_props_patch in the c_*_edgehub_module suites; done means reconnect routes perform the documented client lifecycle and EnableTwin exits clearly when no Twin response arrives.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- api, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100