fix: IPC build reliability and test improvements#27
Merged
Conversation
Update olink core dependency across all version pins: templates, apigear fallback, and test_cmake script.
NATS tests polled _is_ready() via wait_for without subscribing for the readiness callback, causing each wait to hit the full 1000ms timeout. Subscribe to _subscribeForIsReady and notify the condition variable immediately, matching the MQTT pattern. Also reduce thread pool from 10 to 4 workers, add graceful parameter to disconnect(), and lower reply_timeout to 2000ms. Counter module: 30s → 0.09s (~320x speedup).
Remove redundant disconnect wait_for calls in MQTT test teardown. CWrapper::disconnect() fires callbacks synchronously before returning, so the condition is already satisfied. Also reduce test timeout from 2000ms to 1000ms to match other IPC tests.
Replace hardcoded nats_static requirement with target-existence checks that prefer static, fall back to dynamic, and fail if neither is found. Adds MSVC debug-suffix probes and shared-aware Conan target selection.
The MQTT CMakeLists.txt already handles static and dynamic fallback via target-existence checks. The config template should not force a specific component since exported targets encode the linked variant.
The second configure() silently overwrote the first, making fPIC removal for shared builds dead code. Merge both into a single method so fPIC is properly removed when building shared libraries.
The default_options used "nats/*:" patterns but the Conan package reference is "cnats/3.9.1". The mismatch meant options like shared=False were silently ignored, working only because cnats defaults happened to match.
3 tasks
Heap-allocate CallbackContext with its own shared_ptr so the void* pointers cnats holds remain valid after CWrapper destruction. The closedCB (last callback per cnats guarantee) signals a condition variable so the destructor and disconnect wait until all async callbacks have completed before freeing the context. Copy the shared_ptr<natsSubscription> in unsubscribe before releasing the lock — the previous code dereferenced a map iterator after unlock, racing with cleanSubscription on the dispatch thread. Drain thread pools on disconnect to prevent workers from accessing destroyed CWrapper members. Disconnect before destroying adapters so BaseAdapter teardown runs while the connection is still alive. Use readiness notifications in NATS tests to avoid fixed-delay waits.
Replace getPtr() (shared_from_this, throws bad_weak_ptr when expired) with weak_from_this() in all Paho callback contexts. The existing lock() guard in each callback already handles the expired case. Guard onSubscribed/onUnsubscribed callbacks with m_disconnectRequested to prevent invoking adapter callbacks during teardown. Clear all topic maps in disconnect() after joining the run thread so late Paho callbacks find nothing to invoke. Add m_disconnectRequested check and 10-second timeout to waitForPendingMessages() — the previous loop could spin forever because m_connected was set to false only after the thread was joined. Lock m_subscribedTopicsMutex in handleTextMessage() to fix a data race with concurrent modifications. Skip unsubscribeAllTopics() in run() when disconnecting intentionally — MQTTAsync_disconnect handles cleanup.
w4bremer
force-pushed
the
fix/cmake-ipc-reliability
branch
from
6ec1e1b to
d761532
Compare
TSan adds 5-10x overhead to all mutex operations. The hardcoded 1000ms timeout in IPC tests is too tight, causing flaky failures when async round-trips don't complete in time. Use __SANITIZE_THREAD__ (GCC) and __has_feature (Clang) to detect TSan builds and raise the timeout to 10 seconds.
w4bremer
force-pushed
the
fix/cmake-ipc-reliability
branch
from
1ffed6c to
6a19a96
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nats_staticrequirement with static→dynamic→fatal fallback chain, including MSVC debug-suffix probes and shared-aware Conan target selectionpaho-mqtt3as-staticcomponent fromApigearConfig.cmake.in— the CMakeLists.txt already handles the fallbackconfigure()methods (fPIC removal was dead code); correctnats/*option patterns tocnats/*to match the actual package referenceTest plan
ci_generate.yml)ci_build_test.yml)ci_build_features.yml)