[Prior 4] Fix resource leaks, signal handling, and clean lifecycle management in dlt-qnx-system by santhoshsivanhere · Pull Request #917 · COVESA/dlt-daemon

santhoshsivanhere · 2026-09-21T12:22:07Z

Summary

This PR hardens the dlt-qnx-system slogger2 adapter against resource
leaks, undefined behavior, and unclean shutdowns. It replaces heap-based
thread stack allocation with mmap, introduces RAII-based ownership for
DLT contexts and JSON decoders, and reworks signal handling so the daemon
terminates synchronously and predictably.

Changes

1. Resource leaks and exception safety in slogger2

Wrap json_decoder_t with an RAII deleter to guarantee cleanup on all
exit paths in dlt_context_map_read().
Continue parsing remaining entries when JSON object/key extraction fails
instead of breaking early.
Guard against total_size <= 0 in wait_for_buffer_space() to prevent
division by zero.
Explicitly unregister all DltContext entries in clean_qnx_slogger2()
before tearing down storage.
Replace raw pointers in g_slog2file with std::unique_ptr<DltContext>
to enforce exception-safe ownership.

2. Replace malloc-based thread stack allocation with mmap

Replace malloc() / free() for the custom thread stack with mmap()
(MAP_ANONYMOUS | MAP_PRIVATE) and munmap().
Introduce STACK_ALLOC_SIZE so allocation and cleanup use identical
bounds.
Check against MAP_FAILED for accurate error handling.
Removes the need for manual page-alignment calculations before calling
pthread_attr_setstack().

3. Clean lifecycle management and signal handling

Block SIGINT, SIGTERM, SIGHUP, SIGQUIT, and SIGALRM via
pthread_sigmask() before thread creation.
Synchronously wait for termination signals using sigwait() to avoid
async signal-handler deadlocks.
Add atomic flag g_slog2_thread_alive, checked inside
slogger2_callback() and wait_for_buffer_space(), to break out early
on shutdown.
Ensure the mmap-allocated thread stack is cleanly unmapped.
Iterate through g_slog2file to unregister all dynamically registered
DLT contexts on thread exit.
Guarantee pthread_join() completes before calling clean_up() and
unregistering the main DLT application.

Motivation

The slogger2 adapter could leak JSON decoders, dereference dangling
DltContext pointers during teardown, divide by zero on empty buffers,
and terminate uncleanly on signals. Together these fixes make the
lifecycle of the adapter deterministic and exception-safe.

Testing

Built dlt-daemon with -DWITH_DLT_QNX_SYSTEM=ON.
Verified the daemon starts, registers contexts, and shuts down cleanly
on SIGTERM / SIGINT.
Confirmed no leaks reported under the previous code paths.

Fixes #890.

Signed-off-by: Santhosh Sivan Murugan Murugan.SanthoshSivan@in.bosch.com

santhoshsivanhere · 2026-09-21T12:23:25Z

@minminlittleshrimp Please spend some time to review this PR to fix issue #890.

The slogger2 adapter previously suffered from unhandled json_decoder leaks, potential division-by-zero during buffer checks, and unmanaged DltContext pointers that caused dangling references during teardown. Address these stability and resource management issues: - Wrap json_decoder_t with an RAII deleter to guarantee cleanup on all exit paths in dlt_context_map_read(). - Continue parsing remaining entries when JSON object/key extraction fails instead of breaking early. - Guard against total_size <= 0 in wait_for_buffer_space() to prevent division by zero. - Explicitly unregister all DltContext entries in clean_qnx_slogger2() before tearing down storage. - Replace raw pointers in g_slog2file with std::unique_ptr<DltContext> to enforce exception-safe ownership. Signed-off-by: Santhosh Sivan Murugan <santhoshsivanhere@gmail.com> This PR fixes COVESA#890.

Refactor thread stack allocation in dlt-qnx-slogger2-adapter to use mmap() and munmap() instead of malloc() and free(). Using mmap() with MAP_ANONYMOUS | MAP_PRIVATE guarantees proper page-aligned memory allocation directly from the OS, eliminating the need for manual alignment calculations before calling pthread_attr_setstack(). Changes: - Replaced malloc() call with mmap() using PROT_READ | PROT_WRITE. - Replaced free() in free_stackaddr() with munmap() referencing STACK_ALLOC_SIZE. - Defined STACK_ALLOC_SIZE constant to ensure consistent allocation and cleanup bounds. - Checked against MAP_FAILED for accurate error handling. Signed-off-by: Santhosh Sivan Murugan <santhoshsivanhere@gmail.com> This PR fixes COVESA#890.

- Main Loop & Signals: - Block SIGINT, SIGTERM, SIGHUP, SIGQUIT, and SIGALRM via pthread_sigmask prior to thread creation. - Synchronously wait for termination signals using sigwait() to avoid async signal handler deadlocks. - Slogger2 Adapter Lifecycle: - Add atomic flag `g_slog2_thread_alive` check within `slogger2_callback` and `wait_for_buffer_space` to break early on shutdown. - Ensure custom thread stack memory mapped via mmap is cleanly unmapped. - Cleanup Sequence: - Iterate through `g_slog2file` map to unregister all dynamically registered DLT contexts upon thread exit. - Guarantee `pthread_join` completion prior to calling `clean_up()` and unregistering the main DLT application. Signed-off-by: Santhosh Sivan Murugan <santhoshsivanhere@gmail.com> This PR fixes COVESA#890.

minminlittleshrimp · 2026-09-21T16:00:20Z

 extern DltQnxSystemThreads g_threads;

-static std::unordered_map<std::string, DltContext*> g_slog2file;
+static std::unordered_map<std::string, std::unique_ptr<DltContext>> g_slog2file;


DltContext is a C struct, so it still need dlt native api lifecycle (dlt_register_context/unregis context), we can do like this:

struct DltContextDeleter { void operator()(DltContext *ctx) const { if (ctx) { dlt_unregister_context(ctx); delete ctx; } } }; using DltContextPtr = std::unique_ptr<DltContext, DltContextDeleter>;

Then:

static std::unordered_map<std::string, DltContextPtr> g_slog2file;

in cleanup simply:

void clean_qnx_slogger2() { g_slog2file.clear(); // we ok here dltWarnedMissingMappings.clear(); free_stackaddr(); }

just RAII same as Jsondecoderdeleter, what do you think?
Thanks

I dont see dltContextDeleter elsewhere, and DLT_CTX_REG macro still in use

struct DltContextDeleter { void operator()(DltContext *ctx) const { if (ctx) { dlt_unregister_context(ctx); delete ctx; } } };

minminlittleshrimp · 2026-09-21T16:05:03Z

- * Wait for threads to exit.
- */
-static void join_thread()
+/* Point 4: Correct Joining & Context Unregister Sequence */


what it means by point 4? thanks

Implement runtime enable/disable handling for the slog2 adapter. - Add thread-safe runtime enable/disable state management. - Stop, join, clean up, and restart the slog2 thread on injection. - Handle disable/enable race conditions during thread termination. - Add condition-variable based waiting while slog2 is disabled. - Replace slog2 thread stack allocation with mmap and a guard page. - Improve slog2 thread and resource cleanup handling. - Validate slog2 injection service IDs and payloads. - Load slog2 context mappings lazily when the adapter is enabled. - Rename the startup configuration to distinguish it from runtime state. - Improve signal and daemon handling. - Ensure configured DLT IDs are null-terminated. This PR fixes COVESA#890. Signed-off-by: Santhosh Sivan Murugan santhoshsivanhere@gmail.com

minminlittleshrimp · 2026-09-27T16:02:45Z

    sigaddset(&mask, SIGHUP);
    sigaddset(&mask, SIGQUIT);
    sigaddset(&mask, SIGINT);
    sigaddset(&mask, SIGALRM);


Need block also on SIGUSR1?

sigaddset(&mask, SIGUSR1);

Thanks

minminlittleshrimp · 2026-09-27T16:33:56Z

+bool dlt_slog2_is_disabled(void)
+{
+    bool disabled;
+    pthread_mutex_lock(&g_threads.lock);


Should we add definition for lock and cond in header? Thanks

minminlittleshrimp · 2026-09-27T16:34:57Z

        return -1;
    }
+    dlt_set_main_thread(pthread_self());
+    dlt_slog2_set_disabled(!g_dlt_qnx_conf->qnxslogger2.startupEnabled);


where this defined? if it is the enable modified then pls update, thanks

minminlittleshrimp · 2026-09-27T16:35:47Z

-    join_thread();
-
-    if (ret != 0) {
+    if (!dlt_slog2_is_disabled()) {


should this define in header? thanks

minminlittleshrimp · 2026-09-27T16:43:23Z

+ *
+ * Must be called during shutdown after all threads have been joined.
+ */
+int dlt_threads_destroy(void)


yes if this internal func no need to be defined in header, anyway many structs not yet updated?

minminlittleshrimp · 2026-09-27T16:46:58Z

+ */
 void clean_qnx_slogger2()
 {
+    /* RAII Deleter handles dlt_unregister_context() for all map entries automatically */


RAII not completely clean if using macro? should we add:

DLT_UNREGISTER_CONTEXT(dltQnxSlogger2Context); g_adapter_ctx_registered = false?

Here for cleanup? Thanks

minminlittleshrimp · 2026-09-27T17:06:45Z

-
+    auto *conf = static_cast<DltQnxSystemConfiguration*>(param);
    DltLogLevelType loglevel;
    switch (info->severity)


should we check info null? thanks

minminlittleshrimp · 2026-09-27T17:08:41Z

+    else {
+        DLT_LOG(dltQnxSystem, DLT_LOG_ERROR,
+            DLT_STRING("Unknown injection data for slog2 adapter:"),
+            DLT_STRING((char*) data));


bufferoverflow might happen with this data, kindly check, thanks

minminlittleshrimp · 2026-09-27T17:10:57Z

+ * Must be called once before any other dlt_threads/dlt_set/dlt_get functions.
+ * Exits the process on failure.
+ */
+int dlt_threads_init(void)


If pthread_condattr_init, pthread_condattr_setclock, or pthread_cond_init fails after pthread_mutex_init succeeds, the function returns without calling pthread_mutex_destroy?

minminlittleshrimp · 2026-09-27T17:11:21Z

+    pthread_condattr_t cond_attr;
+    ret = pthread_condattr_init(&cond_attr);
+    if (ret != 0) {
+        fprintf(stderr, "pthread_condattr_init failed: %s\n", strerror(ret));


pthread_mutex_destroy(&g_threads.lock);?

minminlittleshrimp · 2026-09-27T17:12:55Z

+ * \return char* A pointer to the first non-whitespace character within the
+ *         original input buffer, or NULL if the input string 's' is NULL.
+ */
+static char *trim_whitespace(char *s)


Where this func is used? if not related here can we remove it? also dlt_parser has these such functions, if we need we can used from that common code, thanks

minminlittleshrimp · 2026-09-27T17:13:39Z


-static void *stackaddr;
+static void *stackaddr = NULL;
+static const size_t STACK_ALLOC_SIZE = PTHREAD_STACK_4K * 4;


Using mmap make this deadcode, should we remove it?

minminlittleshrimp · 2026-09-27T17:14:46Z

+#define STACK_USABLE_SIZE   (PTHREAD_STACK_4K * 64u) /* 256 KiB thread stack */
+#define STACK_TOTAL_SIZE    (STACK_GUARD_SIZE + STACK_USABLE_SIZE)
+static std::atomic_bool g_stop_notified{false};
+static std::atomic_bool g_adapter_ctx_registered{false};


where this bool is checked? if not it a deadcode/bool not-used? thanks

minminlittleshrimp · 2026-09-27T17:16:55Z

@@ -460,4 +826,5 @@ static void clean_up()
        free(g_dlt_qnx_conf->qnxslogger2.contextId);
    if (g_dlt_qnx_conf)
        free(g_dlt_qnx_conf);


is this deadcode? kindly check, thanks (see check line 819 it seems so?)

santhoshsivanhere changed the title ~~dlt-qnx-system: Fix resource leaks and exception safety in slogger2~~ dlt-qnx-system: Fix resource leaks, signal handling, and clean lifecycle management in slogger2 Sep 21, 2026

mft1cob added 3 commits September 21, 2026 20:11

santhoshsivanhere force-pushed the fix/dlt-qnx-system branch from f6a6449 to 580b416 Compare September 21, 2026 14:43

minminlittleshrimp reviewed Sep 21, 2026

View reviewed changes

Comment thread src/dlt-qnx-system/dlt-qnx-system.c

santhoshsivanhere changed the title ~~dlt-qnx-system: Fix resource leaks, signal handling, and clean lifecycle management in slogger2~~ Fix resource leaks, signal handling, and clean lifecycle management in dlt-qnx-system Sep 22, 2026

minminlittleshrimp assigned minminlittleshrimp and santhoshsivanhere and unassigned minminlittleshrimp Sep 22, 2026

minminlittleshrimp added refactor improvement bug labels Sep 22, 2026

minminlittleshrimp changed the title ~~Fix resource leaks, signal handling, and clean lifecycle management in dlt-qnx-system~~ [Prior 4] Fix resource leaks, signal handling, and clean lifecycle management in dlt-qnx-system Sep 22, 2026

minminlittleshrimp reviewed Sep 27, 2026

View reviewed changes

Conversation

santhoshsivanhere commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

1. Resource leaks and exception safety in slogger2

2. Replace malloc-based thread stack allocation with mmap

3. Clean lifecycle management and signal handling

Motivation

Testing

Uh oh!

santhoshsivanhere commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

santhoshsivanhere commented Sep 21, 2026 •

edited

Loading