From: Kienan Stewart <kstewart@efficios.com>
Date: Tue, 16 Jul 2024 13:20:14 +0000 (-0400)
Subject: Fix: Crash when unregistering UST apps during shutdown
X-Git-Tag: v2.13.14~2
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=dd1a5eb168064a053877a91ababc68363c4decf4;p=lttng-tools.git

Fix: Crash when unregistering UST apps during shutdown

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 <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index b43ae58ff..af605052b 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -3283,7 +3283,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);
 
 	}