[haiku-commits] Re: haiku: hrev53089 - src/add-ons/kernel/network/stack
- From: waddlesplash <waddlesplash@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Tue, 23 Apr 2019 09:30:34 -0400
On Tue, Apr 23, 2019, 2:54 AM Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
wrote:
This is still incorrect. You need to move `wait_for_thread`
immediately after change to `ref_count`, otherwise
`device_consumer_thread` can still be in the body of its while loop.
waddlesplash, would you please put your changes through Gerrit instead
of bypassing it systematically?
In this case, Jessica's comment is incorrect; we had a long discussion on
IRC about this. Because fifo_dequeue_buffer is called with
B_INFINITE_TIMEOUT, waiting after setting the ref_count would, in virtually
all cases, simply deadlock as the thread would never wake up.
uninit_fifo destroys the FIFO semaphore and thus unblocks the thread with
B_INTERRUPTED; this is how it exited before. But this did not account for
the case where the thread was not waiting on the semaphore but rather doing
processing of a buffer it dequeued. On this case, after it finished, it
would loop back around and try to dequeue another buffer, but of course the
FIFO had already been destroyed here. So now we check at the beginning of
the loop for that.
Admittedly there is still a problem that this depends on internal FIFO
behavior on destruction, and there is still a small race related to buffer
management. How to resolve that, I don't really know as it would probably
require a much more substantial rework of this code. This change is at
least much more correct and should stop the KDLs people have been getting.
-waddlesplash
Other related posts:
