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

feat: Detect interpreter in shutdown state on thread spawn #2468

Merged
merged 4 commits into from Oct 30, 2023

Conversation

mitsuhiko
Copy link
Member

This detects if the interpreter is already in shutdown state and no longer spawns a background thread.

Fixes #2299

@mitsuhiko mitsuhiko marked this pull request as ready for review October 28, 2023 12:41
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

we have threads in all these places

➜  sentry-python git:(master) ag --depth=0 thread.start sentry_sdk
sentry_sdk/worker.py
70:                self._thread.start()

sentry_sdk/sessions.py
123:            thread.start()

sentry_sdk/profiler.py
921:            self.thread.start()

sentry_sdk/monitor.py
56:            thread.start()

@antonpirker
Copy link
Member

@mitsuhiko I will take over and also add checks for the other places we start threads. Thanks for the contribution!

@antonpirker
Copy link
Member

@sl0thentr0py I will merge this, because it fixes the one place where the problem arises that is mentioned in #2299

I will create another PR for the other places we start threads.

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

lgtm

@antonpirker antonpirker dismissed sl0thentr0py’s stale review October 30, 2023 15:36

The review has a valid point. This PR only fixes one place where threads are started. I will merge this PR and create another one for the other places where we start threads.

@antonpirker antonpirker merged commit e0d7bb7 into master Oct 30, 2023
283 of 284 checks passed
@antonpirker antonpirker deleted the feature/trap-bad-thread-state branch October 30, 2023 15:36
@antonpirker
Copy link
Member

Oh, just saw Ivana already fixed this in our python312 branch.
Ok wurscht, I will make it work.

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

Successfully merging this pull request may close these issues.

Python 3.12 support - RuntimeError
4 participants