Cleanup thread local context in ThreadLocalContextManager after e…
#422
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ach servlet request finishes.
bretthoerner
commented
Jun 20, 2017
| @@ -21,4 +21,9 @@ public Context getContext() { | |||
| return context.get(); | |||
| } | |||
|
|
|||
| @Override | |||
| public void clear() { | |||
| context.remove(); | |||
There was a problem hiding this comment.
This it the critical difference. The rest is dealing with abstractions/naming.
|
I guess it's worth noting that this won't help if a user sets context stuff in a non-servlet thread. I don't have a good solution there if they are loading/unloading/reloading classes. |
tkaemming
approved these changes
Jun 20, 2017
CHANGES
Outdated
| @@ -2,7 +2,8 @@ Version 1.1.1 | |||
| ------------- | |||
|
|
|||
| - Allow overriding the location of the properties file. | |||
| - Add support for handling and uploading Proguard files to Sentry (for Android applications). | |||
| - Add support for handling and uploading Proguard files to Sentry (for Android applications).' | |||
bruno-garcia
pushed a commit
that referenced
this pull request
Jun 12, 2020
…d of scheduled thread executor (#422)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ach servlet request finishes.
This is for #420.
The issue was that threadlocals still held references to classes that were loaded with the old classloader -- and there was nothing to ever remove those old references.
I did some research, and there's no way to remove threadlocals except from the thread that created them (which makes sense). So for now, the only place we do cleanup is in the
SentryServletRequestListener. That should cover most bases, because the most common use of "make a new classloader and reload my entire app" is in application servers that are using servlets.