Skip to content

stream.segmented: join worker+writer on close #4517

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.

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

Merged
merged 2 commits into from
May 8, 2022

Conversation

yhhsin
Copy link
Contributor

@yhhsin yhhsin commented May 7, 2022

A simple, maybe ulgy, attempt to fix #4516 .
There may be more beautiful approaches.

@yhhsin
Copy link
Contributor Author

yhhsin commented May 7, 2022

Another approach:

In SegmentedStreamReader.close(), try to join worker and writer thread, like:

     def close(self):
         self.worker.close()
         self.writer.close()
         self.buffer.close()
+        try:
+            self.writer.join()
+        except RuntimeError:
+            pass
+        try:
+            self.worker.join()
+        except RuntimeError:
+            pass

Note about the try-catch: Since SegmentedStreamReader.close() and SegmentedStreamWriter.close() calls each other, putting join() like above will inevitably join current thread, which causes RuntimeErrors.

Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

The following diff should be enough to fix the issue with the SegmentedStreamWorker and SegmentedStreamWriter deamon threads. No need to add a special method to SegmentedStreamReader, which is just one of the stream (base) classes, and catching an AttributeError when trying to call this method from the CLI on other stream implementations that don't have this method.

diff --git a/src/streamlink/stream/segmented.py b/src/streamlink/stream/segmented.py
index d097be24..26fbdac1 100644
--- a/src/streamlink/stream/segmented.py
+++ b/src/streamlink/stream/segmented.py
@@ -3,7 +3,7 @@ import queue
 from concurrent import futures
 from concurrent.futures import Future, ThreadPoolExecutor
 from sys import version_info
-from threading import Event, Thread
+from threading import Event, Thread, current_thread
 from typing import Any, Optional
 
 from streamlink.buffers import RingBuffer
@@ -228,6 +228,12 @@ class SegmentedStreamReader(StreamIO):
         self.writer.close()
         self.buffer.close()
 
+        current = current_thread()
+        if current is not self.worker:  # pragma: no branch
+            self.worker.join(timeout=self.timeout)
+        if current is not self.writer:  # pragma: no branch
+            self.writer.join(timeout=self.timeout)
+
     def read(self, size):
         return self.buffer.read(
             size,

@bastimeyer bastimeyer changed the title Wait for SegmentedStreamWriter to finish before CLI exits stream.segmented: join worker+writer on close May 8, 2022
@bastimeyer bastimeyer merged commit 402645b into streamlink:master May 8, 2022
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request May 8, 2022
light4 pushed a commit to light4/streamlink that referenced this pull request Dec 9, 2022
Co-authored-by: bastimeyer <mail@bastimeyer.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SegmentedStreamWriter.close() does not reliably finish before CLI exits (race condition)
2 participants