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

Fix: Replace outdated get1Session with getSession in setResume() #229

Closed
wants to merge 3 commits into from

Conversation

gasbytes
Copy link
Contributor

Replaced the call to the outdated native get1Session method with getSession, resolving performance issues for TLS 1.3 session resumption. This change reduces the setResume execution time to well under 2 seconds, meeting performance requirements. All tests passed after the update.

Replaced the call to the outdated native get1Session method with getSession,
resolving performance issues for TLS 1.3 session resumption. This change reduces
the setResume execution time to well under 2 seconds, meeting performance requirements.
All tests passed after the update.
@gasbytes gasbytes assigned wolfSSL-Bot and gasbytes and unassigned wolfSSL-Bot Oct 31, 2024
@cconlon
Copy link
Member

cconlon commented Oct 31, 2024

Hi @gasbytes, I'm not sure this is a change we want to make. I was purposefully calling the native get1Session() because it will check if we have the session ticket, and do a socket select/poll to try and wait for it if it hasn't come in yet. Otherwise, JSSE users were running into issues with getting an incomplete WOLFSSL_SESSION since the ticket hadn't come in yet. We also have a few JNI-only users who are expecting this behavior from the existing WolfSSLSession.getSession() method.

Instead I suggest documenting that users can define WOLFSSL_JNI_DEFAULT_PEEK_TIMEOUT to a different value of their choosing if they want our native select/poll to timeout sooner. That could be added to CFLAGS when compiling the native JNI sources.

For the SunJSSE test in question, I would suggest we not consider the delayed getSession() to be a real error for us.

@gasbytes
Copy link
Contributor Author

Closing this PR as per feedback in this comment.

Thanks Chris!

@gasbytes gasbytes closed this Oct 31, 2024
@gasbytes gasbytes deleted the patch-clientsocketclosehang branch December 30, 2024 18:01
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.

3 participants