Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modifying a stream while running can deadlock #40

Open
bmerry opened this issue Mar 10, 2016 · 2 comments
Open

Modifying a stream while running can deadlock #40

bmerry opened this issue Mar 10, 2016 · 2 comments
Assignees
@bmerry
Labels
bug

Comments

@bmerry
Copy link
Contributor

bmerry commented Mar 10, 2016

The blocking push into the ringbuffer is done with the strand held. If the receiver tries to modify the stream (e.g., to set a memory pool, based on descriptor data) then it can block waiting on the strand, and the ringbuffer will never be emptied. This causes a deadlock.
bmerry added a commit that referenced this issue Mar 10, 2016
@bmerry
Fix some race conditions when changing a running stream
f24a78b 
This partially addresses #40, specifically for set_max_heaps and
set_memory_pool. This is achieved by making these operations atomic (in
the latter case, using a mutex) rather than serialising via the strand.
However, making multiple calls to emplace_reader can still potentially
deadlock if the first reader spams the ringbuffer before the last reader
is added.

Fixing this properly will probably require major architectural changes,
because asio has no way to "drop" the strand during blocking operations.
It probably requires the strand to be abandoned in favour of
fine-grained locking, which may allow for other performance benefits
e.g. using multiple threads to benefit from RSS.
@bmerry bmerry added the bug label May 10, 2016
@bmerry
Copy link
Contributor Author

bmerry commented Mar 5, 2019

Some of these issues have been dealt with by locking, but it can still happen in emplace_reader.

@bmerry bmerry self-assigned this Mar 5, 2019
bmerry added a commit that referenced this issue Mar 6, 2019
@bmerry
Replace run_in_strand with locking
8bf1348 
run_in_strand lead to issues like #40. The new queue_mutex can still be
a source of similar deadlocks, but heap_ready no longer blocks emplace_reader,
because that uses a separate reader_mutex.

It is still possible for deadlocks to occur when sharing a thread pool
between multiple streams with fewer threads than streams: if it has one
thread, it could be blocked waiting for space in ringbuffer A, which can
prevent a stop on B from making progress. Fixing that would require
addressing #30.
bmerry added a commit to ska-sa/katsdppipelines that referenced this issue Mar 6, 2019
@bmerry
Work around for ska-sa/spead2#40
79687ec
@bmerry
Copy link
Contributor Author

bmerry commented Mar 7, 2019

emplace_reader is no longer affected. The only time a user function can be locked out is if it calls flush or one of the functions that calls it (particularly stop). For a single stream that's not an issue with ring_stream because that stops the ringbuffer before stopping the stream, but with a shared thread pool, the threads might be blocked on one stream, and that prevents another stream from stopping because the readers can't run their final handlers.

ludwigschwardt pushed a commit to ska-sa/katsdpcal that referenced this issue Oct 25, 2019
@bmerry
Work around for ska-sa/spead2#40
a57fc11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bmerry bmerry

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bmerry