add timeout to dequeue - unix only#22
Conversation
|
Awesome, thanks! I don't think I'll integrate it as is (especially seeing as it's Linux-only) for now, but I'll be sure to have a look when I get around to adding full support for timeouts. |
|
Hi |
| inline bool wait_dequeue_with_timeout(int64_t timeout_usecs, U& item) | ||
| { | ||
| // 0us is still a timeout, hence >= | ||
| if (timeout_usecs >= 0 && !sema->wait_for_usecs(timeout_usecs)) { |
There was a problem hiding this comment.
There's a bug here; the semaphore must always be decremented before removing something from the queue, even if the timeout is infinite or negative.
|
Apart from being Linux-specific and the if-condition bug, this is pretty close to what I want to implement. I'm going to merge this in locally and use it as a base for the rest. Thanks! |
| m_sema.wait(); | ||
| if (timeout_usecs >= 0) | ||
| { | ||
| return m_sema.wait_for_usecs(timeout_usecs); |
There was a problem hiding this comment.
Ah, this isn't correct. If the wait on the semaphore times out, m_count is still left decremented (indicating that someone is waiting for an element). This essentially imbalances the count of the semaphore after each timeout.
|
Great news. Thanks for looking at this. |
|
Thank you. Let me know of you need me to do something. |
|
Ah, it's much trickier than I thought to add support for timeouts to the LightweightSemaphore. But! I think I have something. I need to test it a bit first, so this is going to take a few more days. Thanks guys for your support! I wouldn't have gotten around to this for quite a while without these pull requests. |
…dan/concurrentqueue.git: add timeout to queue
|
I needed to add #include to get it to build, looking good otherwise. |
|
Cool, thanks. I'll add the header. I haven't written any tests yet, but I'm fairly confident in the logic. |
|
For what it's worth, my app is working fine with your version after swapping the argument order to timed_wait. You might want to consider using std::chrono::duration as the timeout argument rather than integer microseconds, as it's a little friendlier. Maybe as an overload. |
|
Great! I still need tests, all of the tricky code is only executed under special race conditions. |
|
The timed waits seem to work well. If anyone has an issue, please let me know! In particular, this is untested on all platforms except x64 Windows and Linux so far. Consider the feature supported but perhaps be careful using this in production code for now. Cheers everyone! |
3 participants
Only tested on Linux, this won't build for anything else until the semaphore implementations are updated.
The idea is to specify a maximum time to wait for a new item, returning early is ok.
You could also look at adding a c++11 based semaphore implementation, like http://stackoverflow.com/a/27852868 which would support the timeout operations.
This implementation is just using microseconds, but std::chrono could be an alternative too