Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/seedsigner/models/threads.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@
class BaseThread(Thread):
def __init__(self):
super().__init__(daemon=True)

# Intercept the subclass's run() method to catch silent thread crashes
orig_run = self.run
def wrapped_run():
try:
orig_run()
except Exception as e:
logger.exception(f"Thread {self.__class__.__name__} crashed: {e}")
self.keep_running = False
self.run = wrapped_run
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One thought/suggestion that I have is dynamic reassignment of self.run.

Wrapping run() at runtime via self.run = wrapped_run can be a bit fragile and harder to reason about. Would it make sense to instead define a stable run() in BaseThread and delegate subclass logic to something like _run_impl()?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, that is indeed what I was originally intending to do with this PR. However, I realised it would require refactoring across many files. Any class that uses Basethread would have to use _run_impl() instead of run(). Since my main goal was bringing light to this, I did not want to do a big refactor. However, if the maintainers think this a viable problem to work on, I can do those changes too. They would just require a rename of run() to maybe a simpler name like _run() rather than _run_imp() across few files, and a placeholder definition for it with the same logic I have here.


def start(self):
logger.debug(f"{self.__class__.__name__} STARTING")
Expand Down
21 changes: 21 additions & 0 deletions tests/test_threads.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import time
import logging
from seedsigner.models.threads import BaseThread

def test_basethread_crash_handling(caplog):

# This simulates a background process (like hardware listener or scroll ui)
# breaking unexpectedly.
class ExplodingThread(BaseThread):
def run(self):
raise ValueError("Intentional Crash!")

with caplog.at_level(logging.ERROR):
t = ExplodingThread()
t.start()

time.sleep(0.1)

assert getattr(t, 'keep_running', True) is False

assert "Thread ExplodingThread crashed: Intentional Crash!" in caplog.text