[PATCH] [hackrf] Fix hackrf receive hangs by checking before each lock wait
Fix receive path hangs if another thread closes down the hackrf receive whilst this buffer receive function is waiting to be woken up. Now: * Sleep for up to 100ms each time waiting for the cond to be kicked; * Check whether streaming is still enabled each time rather than only when the function is entered. This fixes hangs where consumers like gqrx via gnuradio will do a stop_rx/start_rx very quickly to change something, and the buffer receive path is waiting for a buffer. --- lib/hackrf/hackrf_source_c.cc | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/hackrf/hackrf_source_c.cc b/lib/hackrf/hackrf_source_c.cc index 662d04a..21656f9 100644 --- a/lib/hackrf/hackrf_source_c.cc +++ b/lib/hackrf/hackrf_source_c.cc @@ -30,6 +30,7 @@ #include <stdexcept> #include <iostream> +#include <chrono> #include <gnuradio/io_signature.h> @@ -203,8 +204,22 @@ int hackrf_source_c::work( int noutput_items, { std::unique_lock<std::mutex> lock(_buf_mutex); - while (_buf_used < 3 && running) // collect at least 3 buffers - _buf_cond.wait( lock ); + // XXX adrian - tomorrow, turn this loop into a wait_for (1 second) + // and re-check dev.get / running ! + // Otherwise this will hang reasonably reliably if we race where + // receive is stopped in one thread, and we've already passed + // the hackrf_is_streaming() check; we'll never finish this loop + // and consumers (eg gqrx) will fail. + + while (_buf_used < 3 && running) { // collect at least 3 buffers + _buf_cond.wait_for( lock , std::chrono::milliseconds(100)); + + // Re-check whether the device has closed or stopped streaming + if ( _dev.get() ) + running = (hackrf_is_streaming( _dev.get() ) == HACKRF_TRUE); + else + running = false; + } } if ( ! running ) -- 2.28.0
Err, weirdly I thought I had updated this commit to remove the comment; lemme do that and resend. -a On Mon, 9 Nov 2020 at 10:03, Adrian Chadd <adrian@freebsd.org> wrote:
Fix receive path hangs if another thread closes down the hackrf receive whilst this buffer receive function is waiting to be woken up.
Now:
* Sleep for up to 100ms each time waiting for the cond to be kicked; * Check whether streaming is still enabled each time rather than only when the function is entered.
This fixes hangs where consumers like gqrx via gnuradio will do a stop_rx/start_rx very quickly to change something, and the buffer receive path is waiting for a buffer. --- lib/hackrf/hackrf_source_c.cc | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/lib/hackrf/hackrf_source_c.cc b/lib/hackrf/hackrf_source_c.cc index 662d04a..21656f9 100644 --- a/lib/hackrf/hackrf_source_c.cc +++ b/lib/hackrf/hackrf_source_c.cc @@ -30,6 +30,7 @@
#include <stdexcept> #include <iostream> +#include <chrono>
#include <gnuradio/io_signature.h>
@@ -203,8 +204,22 @@ int hackrf_source_c::work( int noutput_items, { std::unique_lock<std::mutex> lock(_buf_mutex);
- while (_buf_used < 3 && running) // collect at least 3 buffers - _buf_cond.wait( lock ); + // XXX adrian - tomorrow, turn this loop into a wait_for (1 second) + // and re-check dev.get / running ! + // Otherwise this will hang reasonably reliably if we race where + // receive is stopped in one thread, and we've already passed + // the hackrf_is_streaming() check; we'll never finish this loop + // and consumers (eg gqrx) will fail. + + while (_buf_used < 3 && running) { // collect at least 3 buffers + _buf_cond.wait_for( lock , std::chrono::milliseconds(100)); + + // Re-check whether the device has closed or stopped streaming + if ( _dev.get() ) + running = (hackrf_is_streaming( _dev.get() ) == HACKRF_TRUE); + else + running = false; + } }
if ( ! running ) -- 2.28.0
participants (1)
-
Adrian Chadd