Skip to content

Commit

Permalink
Don't nullify on_frame_ready lambda, might cause crash if done during…
Browse files Browse the repository at this point in the history
… user callback
  • Loading branch information
OhadMeir committed Jan 6, 2025
1 parent 1258860 commit 9b91504
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
12 changes: 10 additions & 2 deletions src/dds/rs-dds-sensor-proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,9 @@ void dds_sensor_proxy::add_frame_metadata( frame * const f,

void dds_sensor_proxy::start( rs2_frame_callback_sptr callback )
{
// Remove leftovers from previous starts.
_streaming_by_name.clear();

for( auto & profile : sensor_base::get_active_streams() )
{
auto streamit = _streams.find( sid_index( profile->get_unique_id(), profile->get_stream_index() ) );
Expand All @@ -498,6 +501,7 @@ void dds_sensor_proxy::start( rs2_frame_callback_sptr callback )
invoke_new_frame( static_cast< frame * >( fh.release() ), nullptr, nullptr );
}
} );
streaming.syncer.start();

if( auto dds_video_stream = std::dynamic_pointer_cast< realdds::dds_video_stream >( dds_stream ) )
{
Expand Down Expand Up @@ -547,7 +551,10 @@ void dds_sensor_proxy::stop()
dds_stream->stop_streaming();
dds_stream->close();

_streaming_by_name[dds_stream->name()].syncer.on_frame_ready( nullptr );
// Nullifing the lambda is commented out because we don't want to nullify in middle of user callback (that might
// be long) instead we use start/stop.
//_streaming_by_name[dds_stream->name()].syncer.on_frame_ready( nullptr );
_streaming_by_name[dds_stream->name()].syncer.stop();

if( auto dds_video_stream = std::dynamic_pointer_cast< realdds::dds_video_stream >( dds_stream ) )
{
Expand All @@ -566,7 +573,8 @@ void dds_sensor_proxy::stop()

// Must be done after dds_stream->stop_streaming or we will need to add validity checks to on_data_available,
// and after software_sensor::stop cause to make sure _is_streaming is false
_streaming_by_name.clear();
// Removed here, same reason of killing on_frame_ready lambda instance. Moved to start()
//_streaming_by_name.clear();
}


Expand Down
6 changes: 6 additions & 0 deletions third-party/realdds/include/realdds/dds-metadata-syncer.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>
#include <mutex>
#include <functional>
#include <atomic>


namespace realdds {
Expand Down Expand Up @@ -93,12 +94,17 @@ class dds_metadata_syncer
return frame_holder( frame, _on_frame_release );
}

void start() { _started = true; }
void stop() { _started = false; }

private:
// Call these under lock:
void search_for_match( std::unique_lock< std::mutex > & );
bool handle_match( std::unique_lock< std::mutex > & );
bool handle_frame_without_metadata( std::unique_lock< std::mutex > & );
bool drop_metadata( std::unique_lock< std::mutex > & );

std::atomic< bool > _started;
};


Expand Down
6 changes: 6 additions & 0 deletions third-party/realdds/py/pyrealdds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,12 @@ PYBIND11_MODULE(NAME, m) {
dds_metadata_syncer()
{
on_frame_release( frame_releaser );
start();
}

~dds_metadata_syncer()
{
stop();
}

void enqueue_frame( key_type key, frame_type const & img )
Expand Down
4 changes: 2 additions & 2 deletions third-party/realdds/src/dds-metadata-syncer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ bool dds_metadata_syncer::handle_match( std::unique_lock< std::mutex > & lock )
_metadata_queue.pop_front();
_frame_queue.pop_front();

if( _on_frame_ready )
if( _on_frame_ready && _started )
{
lock.unlock();
_on_frame_ready( std::move( fh ), md );
Expand All @@ -132,7 +132,7 @@ bool dds_metadata_syncer::handle_frame_without_metadata( std::unique_lock< std::
frame_holder fh = std::move( _frame_queue.front().second );
_frame_queue.pop_front();

if( _on_frame_ready )
if( _on_frame_ready && _started )
{
lock.unlock();
_on_frame_ready( std::move( fh ), metadata_type() );
Expand Down

0 comments on commit 9b91504

Please sign in to comment.