From 386dcee2304b127e18233d857c7660db0995e8ad Mon Sep 17 00:00:00 2001 From: Kienan Stewart Date: Tue, 16 Jul 2024 09:20:14 -0400 Subject: [PATCH] Fix: Crash when unregistering UST apps during shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The following crash has been observed in v2.12.2: ``` function=0x55ac7c4c9600 <_ PRETTY FUNCTION .12873> "lttng_ustconsumer_close_metadata") at assert.c:92 function=0x55ac7c4c9600 <_ PRETTY FUNCTION .12873> "lttng_ustconsumer_close_metadata") at assert.c:101 ``` The underlying cause is applicable in the current master branch as well. Cause ===== There is a potential race between the threads the consumerd control thread which handles commands coming from the sessiond and the main thread when shutting down a consumerd. Is it possible that the following happens: 1. `destroy_metadata_stream_ht` has the locks on `consumer_data`, `channel`, `stream` 2. `lttng_ustconsumer_close_all_metadata` looks up the channel and starts to try and acquire a channel lock (`stream->chan->lock`) 3. `destroy_metadata_stream_ht` sets `stream->chan` to `null` 4. `destroy_metadata_stream_ht` releases the `stream`, `channel`, and `consumer_data` locks 5. `lttng_ustconsumer_close_all_metadata` now has the channel lock, and looks up `stream->chan` again to call `destroy_metadata_stream_ht`, and that member is now null Solution ======== Acquire the stream lock after acquiring the channel lock. part 2 follows: don't set stream->chan to null. Known drawbacks =============== None. Change-Id: I1d27ea6ac08f3e7ed4624a8921cffb675be649d2 Signed-off-by: Kienan Stewart Signed-off-by: Jérémie Galarneau --- src/common/ust-consumer/ust-consumer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index d274904e8..65543ddf2 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -3211,7 +3211,9 @@ void lttng_ustconsumer_close_all_metadata(struct lttng_ht *metadata_ht) health_code_update(); pthread_mutex_lock(&stream->chan->lock); + pthread_mutex_lock(&stream->lock); lttng_ustconsumer_close_metadata(stream->chan); + pthread_mutex_unlock(&stream->lock); pthread_mutex_unlock(&stream->chan->lock); } -- 2.34.1