LizardByte / LizardByte/Sunshine
Potential deadlocks due to thread_safe pop() with infinite wait
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 41.3k
- Forks
- 2.1k
- Avg merge
- 23h 47m
- Merged PRs (30d)
- 124
Description
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Is your issue described in the documentation?
- [x] I have read the documentation
### Is your issue present in the latest beta/pre-release?
This issue is present in the latest pre-release
### Describe the Bug
To fix #5249 an issue with unbounded (infinitely waiting) `pop()` was discovered and fixed to stop streams freezing in a `while(peek()) { pop() }` to drain images. The same pattern is also used in other parts of the code that should be checked for potential deadlocks due to waiting infinitely and corrected if possible.
The [codeparts](https://github.com/LizardByte/Sunshine/pull/5249#issuecomment-4796712749) identified using a similar pattern are:
```
$ git grep -nC1 "pop()" | grep -C1 "peek()"
--
src/input.cpp-463- if (touch_port_event->peek()) {
src/input.cpp:464: touch_port = *touch_port_event->pop();
--
--
src/stream.cpp-1117- while (feedback_queue->peek()) {
src/stream.cpp:1118: auto feedback_msg = feedback_queue->pop();
--
--
src/stream.cpp-1124- while (session->control.peer && hdr_queue->peek()) {
src/stream.cpp:1125: auto hdr_info = hdr_queue->pop();
--
--
src/stream.cpp-1197- while (message_queue_queue->peek()) {
src/stream.cpp:1198: auto message_queue_opt = message_queue_queue->pop();
--
src/stream.cpp:1297: while (auto packet = packets->pop()) {
src/stream.cpp-1298- if (shutdown_event->peek()) {
--
--
src/stream.cpp:1620: while (auto packet = packets->pop()) {
src/stream.cpp-1621- if (shutdown_event->peek()) {
--
--
--
src/video.cpp-1420- while (capture_ctx_queue->peek()) {
src/video.cpp:1421: capture_ctxs.emplace_back(std::move(*capture_ctx_queue->pop()));
--
--
src/video.cpp-1482- if (switch_display_event->peek()) {
src/video.cpp:1483: display_p = std::clamp(*switch_display_event->pop(), 0, (int) display_names.size() - 1);
--
--
src/video.cpp-2302- if (switch_display_event->peek()) {
src/video.cpp:2303: display_p = std::clamp(*switch_display_event->pop(), 0, (int) display_names.size() - 1);
--
--
src/video.cpp-2335- while (encode_session_ctx_queue.peek()) {
src/video.cpp:2336: auto encode_session_ctx = encode_session_ctx_queue.pop();
--
tests/unit/test_audio.cpp:55: while (const auto packet = packets->pop()) {
tests/unit/test_audio.cpp-56- if (shutdown_event->peek()) {
--
tests/unit/test_http_pairing.cpp-118- ASSERT_EQ(add_cert->peek(), true);
tests/unit/test_http_pairing.cpp:119: auto cert = add_cert->pop();
```
### Expected Behavior
_No response_
### Additional Context
_No response_
### Host Operating System
other, n/a
### Operating System Version
n/a
### Architecture
amd64/x86_64
### Package
n/a
### GPU Type
n/a
### GPU Model
n/a
### GPU Driver/Mesa Version
n/a
### Capture Method
None
### Apps
```json
```
### Log output
```shell
```
### Online logs
_No response_
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the pop()/peek() locations listed in src/input.cpp, src/stream.cpp, src/video.cpp, tests/unit/test_audio.cpp, and tests/unit/test_http_pairing.cpp, then compare them with the fix for #5249. Check each pattern for an unbounded wait and update the affected code where appropriate; run the relevant unit tests and confirm stream draining no longer risks deadlock.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100