Skip to content
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

Potential "cannot join thread before it is started" when shutting down during startup #626

Open
1 of 3 tasks
kasium opened this issue Apr 5, 2023 · 4 comments
Open
1 of 3 tasks
Labels
bug Something is broken triage

Comments

@kasium
Copy link
Contributor

kasium commented Apr 5, 2023

❓ I'm submitting a ...

  • 🐞 bug report
  • 🐣 feature request
  • ❓ question about the decisions made in the repository

🐞 Describe the bug. What is the current behavior?
If during startup of cheroot while the worker threads are not yet started a shutdown is requested (e.g. by a signal handler), the thread pool might run into "cannot join thread before it is started".

Exact line:

worker.join(remaining_time)

I guess that _clear_threads should check the current state of the thread:

def _clear_threads(self):
"""Clear self._threads and yield all joinable threads."""
# threads = pop_all(self._threads)
threads, self._threads[:] = self._threads[:], []
return (
thread
for thread in threads
if thread is not threading.current_thread()
)

❓ What is the motivation / use case for changing the behavior?
Have a clean shutdown routine even in corner cases

πŸ’‘ To Reproduce
n/a

πŸ’‘ Expected behavior
Not yet started threads should not be joined

πŸ“‹ Details
n/a

πŸ“‹ Environment

  • Cheroot version: 8.4.1
  • Python version: 3.11.1
  • OS: Linux
@kasium kasium added bug Something is broken triage labels Apr 5, 2023
@webknjaz
Copy link
Member

webknjaz commented May 9, 2023

Could you post a more detailed reproducer and a traceback? Or maybe add a PR with a test case for this?

@kasium
Copy link
Contributor Author

kasium commented May 11, 2023

Not very nice, but here is a simple example

from threading import Event, Thread
from flask import Flask
from cheroot.wsgi import Server
from cheroot.workers.threadpool import WorkerThread

app = Flask("sample")
server = Server(("localhost", 5001), app)
stop_done = Event()
in_start = Event()

original_start = WorkerThread
def start(*args, **kwargs):
    in_start.set()
    stop_done.wait()
    original_start(*args, **kwargs)
WorkerThread.start = start

def target():
    in_start.wait()
    server.stop()
    stop_done.set()

thread = Thread(target=target)
thread.start()
server.safe_start()
thread.join()

Stacktrace

Exception in thread Thread-1 (target):
Traceback (most recent call last):
  File "/root/.pyenv/versions/3.11.0/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/root/.pyenv/versions/3.11.0/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/root/foo.py", line 21, in target
    server.stop()
  File "/root/venv/lib/python3.11/site-packages/cheroot/server.py", line 2094, in stop
    self.requests.stop(self.shutdown_timeout)
  File "/root/venv/lib/python3.11/site-packages/cheroot/workers/threadpool.py", line 292, in stop
    worker.join(remaining_time)
  File "/root/.pyenv/versions/3.11.0/lib/python3.11/threading.py", line 1107, in join
    raise RuntimeError("cannot join thread before it is started")
RuntimeError: cannot join thread before it is started

Instead of the monkeypatching, now image that exactly before WorkerThread.start is called a signal occurs. The signal handler in my case will then ask the server to stop. This leads to the original issuue

@webknjaz
Copy link
Member

I suppose this snipped could be further simplified with

- from flask import Flask
- app = Flask("sample")
- server = Server(("localhost", 5001), app)
+ server = Server(("localhost", 5001), lambda *_, **__: None)

right?

@kasium
Copy link
Contributor Author

kasium commented May 11, 2023

yes, this also works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken triage
Projects
None yet
Development

No branches or pull requests

2 participants