From fbbfcdb7a6d1063859440771a991fbd018f40dd6 Mon Sep 17 00:00:00 2001
From: Alexander Lerach <alexander@auterion.com>
Date: Mon, 11 Nov 2024 14:23:46 +0100
Subject: [PATCH] uxrce_dds_client - Fix nullptr access in certain cleanup
 situations (#23913)

* Only close initialized objects

* Added err log in case of no ping

* Added err log in case of timesync timeout
---
 .../uxrce_dds_client/uxrce_dds_client.cpp       | 17 +++++++++++++++--
 src/modules/uxrce_dds_client/uxrce_dds_client.h |  4 +++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/modules/uxrce_dds_client/uxrce_dds_client.cpp b/src/modules/uxrce_dds_client/uxrce_dds_client.cpp
index 690ae53af1..339bedd7a9 100644
--- a/src/modules/uxrce_dds_client/uxrce_dds_client.cpp
+++ b/src/modules/uxrce_dds_client/uxrce_dds_client.cpp
@@ -222,6 +222,7 @@ bool UxrceddsClient::setup_session(uxrSession *session)
 	}
 
 	if (!got_response) {
+		PX4_ERR("got no ping from agent");
 		return false;
 	}
 
@@ -241,6 +242,8 @@ bool UxrceddsClient::setup_session(uxrSession *session)
 		return false;
 	}
 
+	_session_created = true;
+
 	// Streams
 	// Reliable for setup, afterwards best-effort to send the data (important: need to create all 4 streams)
 	_reliable_out = uxr_create_output_reliable_stream(session, _output_reliable_stream_buffer,
@@ -351,6 +354,7 @@ bool UxrceddsClient::setup_session(uxrSession *session)
 		}
 
 		if (sync_timeouts > TIMESYNC_MAX_TIMEOUTS) {
+			PX4_ERR("timeout during time synchronization");
 			return false;
 		}
 
@@ -378,10 +382,18 @@ bool UxrceddsClient::setup_session(uxrSession *session)
 void UxrceddsClient::delete_session(uxrSession *session)
 {
 	delete_repliers();
-	uxr_delete_session_retries(session, _connected ? 1 : 0);
+
+	if (_session_created) {
+		uxr_delete_session_retries(session, _connected ? 1 : 0);
+		_session_created = false;
+	}
+
+	if (_subs_initialized) {
+		_subs->reset();
+		_subs_initialized = false;
+	}
 
 	_last_payload_tx_rate = 0;
-	_subs->reset();
 	_timesync.reset_filter();
 }
 
@@ -549,6 +561,7 @@ void UxrceddsClient::run()
 		int poll_error_counter = 0;
 
 		_subs->init();
+		_subs_initialized = true;
 
 		while (!should_exit() && _connected) {
 			perf_begin(_loop_perf);
diff --git a/src/modules/uxrce_dds_client/uxrce_dds_client.h b/src/modules/uxrce_dds_client/uxrce_dds_client.h
index d00571d3f9..2a75d73e4a 100644
--- a/src/modules/uxrce_dds_client/uxrce_dds_client.h
+++ b/src/modules/uxrce_dds_client/uxrce_dds_client.h
@@ -181,9 +181,11 @@ private:
 
 	int _last_payload_tx_rate{}; ///< in B/s
 	int _last_payload_rx_rate{}; ///< in B/s
-	bool _connected{false};
 
+	bool _connected{false};
+	bool _session_created{false};
 	bool _timesync_converged{false};
+	bool _subs_initialized{false};
 
 	Timesync _timesync{timesync_status_s::SOURCE_PROTOCOL_DDS};