-
Notifications
You must be signed in to change notification settings - Fork 1k
PlayWS strict context check #4365
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
PlayWS strict context check #4365
Conversation
| @Advice.OnMethodEnter(suppress = Throwable.class) | ||
| public static Scope onEnter() { | ||
| if (Java8BytecodeBridge.currentContext() != Java8BytecodeBridge.rootContext()) { | ||
| return Java8BytecodeBridge.rootContext().makeCurrent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it forcefully removes the current context for while the method executes; this way preventing the context leaking into a (potentially long-running) task that's started there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will everybody understand that in a year while reading this code? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait a year and see 🤣
But yeah, a comment here explaining why we're doing such a thing would be invaluable.
| @Advice.OnMethodEnter(suppress = Throwable.class) | ||
| public static Scope onEnter() { | ||
| if (Java8BytecodeBridge.currentContext() != Java8BytecodeBridge.rootContext()) { | ||
| return Java8BytecodeBridge.rootContext().makeCurrent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it forcefully removes the current context for while the method executes; this way preventing the context leaking into a (potentially long-running) task that's started there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* PlayWS strict context check * add comment
Part of #3916