rmannibucau commented on a change in pull request #34:
URL: https://github.com/apache/openwebbeans/pull/34#discussion_r632500938
##########
File path:
webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java
##########
@@ -593,9 +593,17 @@ protected boolean sessionIsExpiring(HttpSession session)
int maxInactiveInterval = session.getMaxInactiveInterval();
if (maxInactiveInterval > 0)
{
- long inactiveSince =
TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() -
session.getLastAccessedTime());
- if (inactiveSince >= maxInactiveInterval)
+ try
{
+ long inactiveSince =
TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() -
session.getLastAccessedTime());
+ if (inactiveSince >= maxInactiveInterval)
+ {
+ return true;
+ }
+ }
+ catch (IllegalStateException e)
+ {
+ // Jetty will throw an ISE if you attempt to query the last
accessed time of a session that is being invalidated
return true;
Review comment:
> @rmannibucau I was not clear, let me try again.
>
> If the session is being "invalidated" (aka then about to be invalid) then
you are correct `getLastAccedTime()` should not throw an exception as you say.
> However the method that is being patched here is as follows:
>
>
https://github.com/apache/openwebbeans/blob/e9332767af5abd81867b21b8a4d10718733fb188/webbeans-web/src/main/java/org/apache/webbeans/web/context/WebContextsService.java#L588-L603
>
> The javadoc for this method _clearly_ states (with some typos)
>
> > @return {@code true} if the sessino is currently expiring or has already
expired
>
> thus - either this javaodoc comment is wrong - as it must never be called
by a session that has **already** expired (aka has already been invalidated) .
> or
> the code is buggy as stated as if called with a session that has already
expired, then it will never return `true` but is guaranteed by the
specification to throw an `IllegalArgumentException` for every implementation
that adheres to the specification.
nop, if the session expired we got the listener called and session context
is no more available so in terms of codepath it shouldn't be possible (mainly
because of the links I sent previously).
The two cases we want to cover are:
* destroying the session on timeout (listener case, no issue calling this
method)
* destroying it after the request (end of service()) when invalidate() is
called manually in the session during the request
In both cases the session is not yet invalidated so is fully functional in
the listener.
@tandraschko @stephenc
org.apache.webbeans.web.context.WebContextsService#destroySessionImmediately
looks like a toggle waiting for its configuration to be wired ;). Happy if this
one and/or friends are wired here or in jetty module (kind of follow strict
servlet lifecycle vs default cdi lifecycle).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
