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
PX4 · Nov 11, 2024 · fbbfcdb · fbbfcdb
1 parent 3e3151c
commit fbbfcdb
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 3 deletions.
diff --git 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
@@ -181,9 +181,11 @@ class UxrceddsClient : public ModuleBase<UxrceddsClient>, public ModuleParams
 
 	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};