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.

Sign up for GitHub

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

Jump to bottom

Allow blocking calls in JdkSslContext class initializer #12435

Merged
merged 2 commits into from Jun 1, 2022
Merged

Allow blocking calls in JdkSslContext class initializer #12435

merged 2 commits into from Jun 1, 2022

Conversation

chrisvest
Copy link
Contributor

Motivation:
Classes might be initialized inside the event loop where blocking calls are otherwise forbidden.
The JdkSslContext has a blocking call in its class initializer through initializing the Java SSLContext which does IO to deserialize its key store.

Modification:
Collect the class initialization code in a method, and allow it to have blocking calls in blockhound.

Result:
No more blockhound warnings about blocking calls in JdkSslContext class initializer.

Fixes #12425

@chrisvest
Allow blocking calls in JdkSslContext class initializer
7f5fe0d 
Motivation:
Classes might be initialized inside the event loop where blocking calls are otherwise forbidden.
The JdkSslContext has a blocking call in its class initializer through initializing the Java SSLContext which does IO to deserialize its key store.

Modification:
Collect the class initialization code in a method, and allow it to have blocking calls in blockhound.

Result:
No more blockhound warnings about blocking calls in JdkSslContext class initializer.

Fixes netty#12425
@chrisvest chrisvest added this to the 4.1.78.Final milestone May 31, 2022
@normanmaurer
Copy link
Member

Please fix the exception:

Caused by: java.lang.NullPointerException
[4620](https://github.com/netty/netty/runs/6679954447?check_suite_focus=true#step:5:4621)
	at java.base/java.util.ArrayList.<init>(ArrayList.java:179)
[4621](https://github.com/netty/netty/runs/6679954447?check_suite_focus=true#step:5:4622)
	at io.netty.handler.ssl.JdkSslContext$Defaults.init(JdkSslContext.java:113)
[4622](https://github.com/netty/netty/runs/6679954447?check_suite_focus=true#step:5:4623)
	at io.netty.handler.ssl.JdkSslContext.<clinit>(JdkSslContext.java:73)
[4623](https://github.com/netty/netty/runs/6679954447?check_suite_focus=true#step:5:4624)
	... 67 more

normanmaurer
normanmaurer requested changes Jun 1, 2022
View reviewed changes
Copy link
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

Suggested a fix for the NPE

handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java Outdated Show resolved Hide resolved
handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java Outdated Show resolved Hide resolved
@chrisvest
Copy link
Contributor Author

@normanmaurer D'oh. Thanks for the fixes :)

@chrisvest chrisvest merged commit 56f7c50 into netty:4.1 Jun 1, 2022
@chrisvest chrisvest deleted the 41-jdksslcontext-blockhound branch June 1, 2022 16:17
chrisvest added a commit that referenced this pull request Jun 1, 2022
@chrisvest @normanmaurer
Allow blocking calls in JdkSslContext class initializer (#12435)
2cd0b4c 
Motivation:
Classes might be initialized inside the event loop where blocking calls are otherwise forbidden.
The JdkSslContext has a blocking call in its class initializer through initializing the Java SSLContext which does IO to deserialize its key store.

Modification:
Collect the class initialization code in a method, and allow it to have blocking calls in blockhound.

Result:
No more blockhound warnings about blocking calls in JdkSslContext class initializer.

Fixes #12425

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
@chrisvest @normanmaurer
Allow blocking calls in JdkSslContext class initializer (netty#12435)
fce9051 
Motivation:
Classes might be initialized inside the event loop where blocking calls are otherwise forbidden.
The JdkSslContext has a blocking call in its class initializer through initializing the Java SSLContext which does IO to deserialize its key store.

Modification:
Collect the class initialization code in a method, and allow it to have blocking calls in blockhound.

Result:
No more blockhound warnings about blocking calls in JdkSslContext class initializer.

Fixes netty#12425

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
franz1981 pushed a commit to franz1981/netty that referenced this pull request Aug 22, 2022
@chrisvest @normanmaurer @franz1981
Allow blocking calls in JdkSslContext class initializer (netty#12435)
d2c52b0 
Motivation:
Classes might be initialized inside the event loop where blocking calls are otherwise forbidden.
The JdkSslContext has a blocking call in its class initializer through initializing the Java SSLContext which does IO to deserialize its key store.

Modification:
Collect the class initialization code in a method, and allow it to have blocking calls in blockhound.

Result:
No more blockhound warnings about blocking calls in JdkSslContext class initializer.

Fixes netty#12425

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@normanmaurer normanmaurer normanmaurer requested changes

@Kvicii Kvicii Kvicii approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.1.78.Final
Development

Successfully merging this pull request may close these issues.

Jdk sslhandler have blocking call
3 participants
@chrisvest @normanmaurer @Kvicii