applications: nrf_audio: Move conn unref to after zbus publish by alexsven · Pull Request #30798 · nrfconnect/sdk-nrf

alexsven · 2026-08-19T14:38:40Z

Move the unref of a conn pointer to after it has been published
- Increases chance of the conn pointer being valid
Document that the conn pointer might be unvalid
Add checks to make sure conn pointer is valid where it is used
OCT-3490

NordicBuilder · 2026-08-19T14:42:42Z

CI Information

^{To view the history of this post, click the 'edited' button above}
Build number: 3

Inputs:

Sources:

sdk-nrf: PR head: 2c0ecba7b12d7678f8f5ec1f3033a94eb371f307

more details

sdk-nrf:

PR head: 2c0ecba7b12d7678f8f5ec1f3033a94eb371f307
merge base: 15b36cb022b160e024277df4fbd3bbe606051960
target head (main): 40a978d1dd14e279c0fe6fed3530364d5582efea
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister
	ci-run-draft	Run CI on draft pull requests

List of changed files detected by CI (4)

applications
│  ├── nrf_audio
│  │  ├── include
│  │  │  │ zbus_common.h
│  │  ├── src
│  │  │  ├── bluetooth
│  │  │  │  ├── bt_management
│  │  │  │  │  │ bt_mgmt.c
│  │  ├── unicast_client
│  │  │  │ main.c
│  │  ├── unicast_server
│  │  │  │ main.c

Outputs:

Toolchain

Version: e214c2a47c
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:e214c2a47c_a140d687bb

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
- sdk-nrf test count: 25
✅ Integration tests
- ✅ test-sdk-audio

Disabled integration tests

- test-fw-nrfconnect-nrf_lrcs_mosh
- test-fw-nrfconnect-nrf_lrcs_positioning
- desktop52_verification
- test_ble_nrf_config
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-apps_nrfdesktop
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-ble_samples
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread-main
- test-low-level
- test-sdk-dfu
- test-sdk-find-my
- test-sdk-mcuboot
- test-sdk-wifi
- test-sdk-wifi-net

Note: This message is automatically posted and updated by the CI

Copilot

Pull request overview

This PR adjusts Bluetooth disconnection event handling in the nRF Audio application so that the bt_conn pointer is less likely to be invalid when delivered via zbus, and updates consumers/documentation accordingly.

Changes:

Move bt_conn_unref() in disconnected_cb() to occur after publishing the zbus BT_MGMT_DISCONNECTED event.
Add defensive handling in unicast client/server disconnection listeners to retain the connection during processing.
Document bt_mgmt_msg.conn lifetime expectations for zbus consumers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
applications/nrf_audio/unicast_server/main.c	Adds disconnection-event conn validation / temporary ref management in the zbus listener.
applications/nrf_audio/unicast_client/main.c	Adds disconnection-event conn validation / temporary ref management in the zbus listener.
applications/nrf_audio/src/bluetooth/bt_management/bt_mgmt.c	Moves `bt_conn_unref()` to after zbus publish in `disconnected_cb()`.
applications/nrf_audio/include/zbus_common.h	Documents expected `conn` pointer lifetime for bt_mgmt zbus messages.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

applications/nrf_audio/include/zbus_common.h:94

These two lines have an extra leading space before the indentation tab, unlike the rest of this Doxygen block (zbus_common.h:91-92,95-97). Remove it to keep the comment consistently indented and checkpatch-clean.

 	 *
 	 * If a listener needs to keep using this pointer beyond the immediate zbus listener

Thalley · 2026-08-19T15:09:39Z

 	ret = zbus_chan_pub(&bt_mgmt_chan, &msg, K_NO_WAIT);
 	ERR_CHK(ret);

+	if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
+		bt_conn_unref(conn);
+	}


This isn't a thread safe solution.

bt_conn_unref(conn); may still be called before any of the subscribers for the message gets it. The only way to ensure that conn is valid in the callbacks, is that if bt_conn_ref is being called for each subscriber here. I'm not sure if that's possible though.

Is is possible to flush the channel, so that you can be sure that all callbacks are called before bt_conn_unref?

We are aware that it is not thread safe, this was seen as the best possible solution as we don't necessarily control if the recipient of the message is a listeners or a subscriber.

Oh, nvm, read a bit more up on zbus_chan_pub, and that is a synchronous API (i.e. the callbacks of the subscribers are called directly in this thread), so this should be working fine.

@Thalley : That may well be the case for listeners, but I don't think that is the case for subscribers.
"Message subscribers, a thread-based observer that relies internally on a FIFO where the event dispatcher puts a copy of the message every time an observed channel is published or notified."

Another way which I believe is better:

When a subscriber needs to use the conn, do a:
struct bt_conn = bt_conn_ref(conn);

If bt_conn != NULL; then the ref is OK.
The only thing which may theoretically occur, is that the conn changes, so the same conn is actually re-used for a different conn.

- Move the unref of a conn pointer to after it has been published - Increases chance of the conn pointer being valid - Document that the conn pointer might be unvalid - Add checks to make sure conn pointer is valid where it is used - OCT-3490 Signed-off-by: Alexander Svensen <alexander.svensen@nordicsemi.no>

github-actions · 2026-08-20T06:06:56Z

You can find the documentation preview for this PR here.

Thalley · 2026-08-20T06:39:26Z

 	ret = zbus_chan_pub(&bt_mgmt_chan, &msg, K_NO_WAIT);
 	ERR_CHK(ret);

+	if (IS_ENABLED(CONFIG_BT_CENTRAL)) {
+		bt_conn_unref(conn);
+	}


Oh, nvm, read a bit more up on zbus_chan_pub, and that is a synchronous API (i.e. the callbacks of the subscribers are called directly in this thread), so this should be working fine.

Thalley · 2026-08-20T06:42:49Z

+		bt_conn_ref(msg->conn);
+
 		ret = bt_content_ctrl_conn_disconnected(msg->conn);
 		if (ret) {
 			LOG_ERR("Failed to handle disconnection in content control: %d", ret);
 		}

+		bt_conn_unref(msg->conn);


Since zbus_chan_pub is synchronous for ZBUS_LISTENER_DEFINE we don't need to do this bt_conn_ref and bt_conn_unref here, as the reference from the disconnected_cb is still valid.

It's only if the observer type was any other type that you'd (possibly) need this.

Suggested change

bt_conn_ref(msg->conn);

ret = bt_content_ctrl_conn_disconnected(msg->conn);

if (ret) {

LOG_ERR("Failed to handle disconnection in content control: %d", ret);

}

bt_conn_unref(msg->conn);

ret = bt_content_ctrl_conn_disconnected(msg->conn);

if (ret) {

LOG_ERR("Failed to handle disconnection in content control: %d", ret);

}

Thalley · 2026-08-20T06:43:09Z

+		if (msg->conn == NULL) {
+			LOG_ERR("Disconnected event with NULL conn");
+			return;
+		}
+
+		bt_conn_ref(msg->conn);
 		unicast_client_conn_disconnected(msg->conn);
+		bt_conn_unref(msg->conn);



Suggested change

if (msg->conn == NULL) {

LOG_ERR("Disconnected event with NULL conn");

return;

}

bt_conn_ref(msg->conn);

unicast_client_conn_disconnected(msg->conn);

bt_conn_unref(msg->conn);

if (msg->conn == NULL) {

LOG_ERR("Disconnected event with NULL conn");

return;

}

unicast_client_conn_disconnected(msg->conn);

Thalley · 2026-08-20T06:43:46Z

+		if (msg->conn == NULL) {
+			LOG_ERR("Disconnected event with NULL conn");
+			return;
+		}


For the same reason as mentioned elsewhere, this check is unnecessary if this is only triggered from disconnected_cb. It's fine to keep as is though

koffes · 2026-08-20T11:50:05Z

+			return;
+		}
+
+		bt_conn_ref(msg->conn);


Need to check that the return from this is non-NULL.

The same applies above.

alexsven requested a lite review from Copilot August 19, 2026 14:38

alexsven added the no-changelog label Aug 19, 2026

Copilot started reviewing on behalf of alexsven August 19, 2026 14:39 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Comment thread applications/nrf_audio/unicast_server/main.c Outdated

Comment thread applications/nrf_audio/unicast_client/main.c Outdated

Comment thread applications/nrf_audio/include/zbus_common.h

alexsven force-pushed the OCT-3490-Conn-pointer-unrefed-and-then-used branch from 5419c25 to be4a550 Compare August 19, 2026 14:52

alexsven requested a balanced review from Copilot August 19, 2026 14:53

Copilot started reviewing on behalf of alexsven August 19, 2026 14:53 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

alexsven marked this pull request as ready for review August 19, 2026 15:00

alexsven requested a review from a team as a code owner August 19, 2026 15:00

alexsven requested review from Thalley, gWacey and koffes August 19, 2026 15:00

Thalley reviewed Aug 19, 2026

View reviewed changes

alexsven force-pushed the OCT-3490-Conn-pointer-unrefed-and-then-used branch from be4a550 to 2c0ecba Compare August 20, 2026 05:33

Thalley requested changes Aug 20, 2026

View reviewed changes

koffes requested changes Aug 20, 2026

View reviewed changes

Uh oh!

Conversation

alexsven commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NordicBuilder commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

github-actions Bot commented Aug 20, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alexsven commented Aug 19, 2026 •

edited

Loading

NordicBuilder commented Aug 19, 2026 •

edited

Loading