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

Issue #4888 - Request getSession() now returns existing cached session if found #4889

Closed
wants to merge 1 commit into from

Conversation

cshannon
Copy link

Instead of logging and exception and returning null, if an existing
Session is found in the session cache when attempting to add a new
session then the existing session is used instead and returned

…ession if found

Instead of logging and exception and returning null, if an existing
Session is found in the session cache when attempting to add a new
session then the existing session is used instead and returned

Signed-off-by: Christopher L. Shannon <christopher.l.shannon@gmail.com>
@joakime joakime requested a review from janbartel May 19, 2020 14:07
@joakime joakime added this to In progress in Jetty 9.4.30 via automation May 19, 2020
@joakime joakime moved this from In progress to Review in progress in Jetty 9.4.30 May 19, 2020
@joakime joakime removed this from Review in progress in Jetty 9.4.30 Jun 12, 2020
@joakime joakime added this to In progress in Jetty 9.4.31 via automation Jun 12, 2020
@joakime joakime moved this from In progress to Review in progress in Jetty 9.4.31 Jun 12, 2020
@joakime joakime marked this pull request as draft July 22, 2020 19:55
@joakime
Copy link
Contributor

joakime commented Jul 22, 2020

Since @janbartel marked this as "do not merge" , i've downgraded this from a formal PR to a draft PR until her questions are answered.

@joakime
Copy link
Contributor

joakime commented Jul 22, 2020

Note: we are close to having a 9.4.31 release (next week or two), so if you want to see this change in 9.4.31, I would advise you to resolve the unresolved questions that @janbartel has.

@joakime joakime moved this from Review in progress to In progress in Jetty 9.4.31 Jul 22, 2020
@joakime joakime removed this from In progress in Jetty 9.4.31 Jul 23, 2020
@joakime joakime added this to In progress in Jetty 9.4.32 via automation Jul 23, 2020
Jetty 9.4.32 automation moved this from In progress to Review in progress Sep 1, 2020
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

This definitely feels like treating the symptom and not the cause. Let's put this on hold until we understand the issue more.

@janbartel
Copy link
Contributor

@cshannon I've just committed a change with PR #5220 which I think might resolve your issue also. So I'm going to close this PR for now. If you can retest with HEAD of jetty-9.4 and confirm if that resolves your problem over on issue #4888 that would be great.

@janbartel janbartel closed this Sep 2, 2020
Jetty 9.4.32 automation moved this from Review in progress to Done Sep 2, 2020
@cshannon
Copy link
Author

cshannon commented Sep 2, 2020

@janbartel - Thanks a lot, I will give it a shot but I think you are right that this will fix the issue. I have been running a custom build for a while now to fix it for myself but I'm glad that some others were able to report back a test case to demonstrate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Jetty 9.4.32
  
Done
Development

Successfully merging this pull request may close these issues.

Request getSession() method throws IllegalStateException when Session exists
4 participants