It seems WebSession's removeAttribute does not support HttpSessionBindingListener

[yucigou]

When a session expires or is invalidated, or a session attribute gets removed, etc., HttpSessionBindingListener's valueUnbound callback function should be fired.

However, it seems that WebSession's removeAttribute does not support HttpSessionBindingListener. (I'm referring to the Ignite Web module.)

class WebSession implements HttpSession, Externalizable {

    /** {@inheritDoc} */
    @Override public void removeAttribute(String name) {
        if (!isValid)
            throw new IllegalStateException("Call on invalidated session!");

        attrs.remove(name);

        if (updates != null)
            updates.add(new T2<>(name, null));
    }

...

Somehow, our application relies on the HttpSessionBindingListener's valueUnbound callback function getting called to clean up resources.

Any advice please?

PS.:
Tomcat's implementation of HttpSession looks like:

StandardSession.java

    protected void removeAttributeInternal(String name, boolean notify) {
        // Avoid NPE
        if (name == null) return;
        // Remove this attribute from our collection
        Object value = attributes.remove(name);
        // Do we need to do valueUnbound() and attributeRemoved() notification?
        if (!notify || (value == null)) {
            return;
        }
        // Call the valueUnbound() method if necessary
        HttpSessionBindingEvent event = null;
        if (value instanceof HttpSessionBindingListener) {
            event = new HttpSessionBindingEvent(getSession(), name, value);
            ((HttpSessionBindingListener) value).valueUnbound(event);
        }
...

[Valentin Kulichenko]

Hi,

Good catch! I created a ticket for this:
https://issues.apache.org/jira/browse/IGNITE-5607

Are you willing to pick it up and contribute the fix?

-Val

On Wed, Jun 28, 2017 at 3:25 AM, yucigou <[hidden email]> wrote:

> When a session expires or is invalidated, or a session attribute gets
> removed, etc., HttpSessionBindingListener's valueUnbound callback function
> should be fired.
>
> However, it seems that WebSession's removeAttribute does not support
> HttpSessionBindingListener. (I'm referring to the Ignite Web module.)
>
> class WebSession implements HttpSession, Externalizable {
>
>     /** {@inheritDoc} */
>     @Override public void removeAttribute(String name) {
>         if (!isValid)
>             throw new IllegalStateException("Call on invalidated
> session!");
>
>         attrs.remove(name);
>
>         if (updates != null)
>             updates.add(new T2<>(name, null));
>     }
>
> ...
>
> Somehow, our application relies on the HttpSessionBindingListener's
> valueUnbound callback function getting called to clean up resources.
>
> Any advice please?
>
> PS.:
> Tomcat's implementation of HttpSession looks like:
>
> StandardSession.java
>
>     protected void removeAttributeInternal(String name, boolean notify) {
>         // Avoid NPE
>         if (name == null) return;
>         // Remove this attribute from our collection
>         Object value = attributes.remove(name);
>         // Do we need to do valueUnbound() and attributeRemoved()
> notification?
>         if (!notify || (value == null)) {
>             return;
>         }
>         // Call the valueUnbound() method if necessary
>         HttpSessionBindingEvent event = null;
>         if (value instanceof HttpSessionBindingListener) {
>             event = new HttpSessionBindingEvent(getSession(), name,
> value);
>             ((HttpSessionBindingListener) value).valueUnbound(event);
>         }
> ...
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/It-seems-WebSession-s-
> removeAttribute-does-not-support-HttpSessionBindingListener-tp19184.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>

[yucigou]

Thanks Val. Yes, I'll give it a try and test/verify with our app. Get back soon.

[yucigou]

Ignite WebSessionV2 uses genuineSes as the original HttpSession.

Therefore, when setting an attribute or setting the maxInactiveInterval, Ignite should tell the original HttpSession about it.

Otherwise, when the web container (such as Tomcat) thinks that a session expires, or is invalidated, or a session attribute gets removed, etc., session attributes' HttpSessionBindingListener's valueUnbound callback function will not get fired.

So once the original HttpSession gets updated with the session attributes and the maxInactiveInterval, the web container will transitively trigger the session attributes' HttpSessionBindingListener's valueUnbound callback function when a session expires, etc.

(By the way, tested with our app, and our issue is fixed: https://github.com/apache/ignite/pull/2243)

[Valentin Kulichenko]

Hi,

This fix seems to address only particular case when attribute is expired.
But this will not work in generic case, right? For example, if attribute is
put and removed explicitly, listener will not be invoked. I don't think we
should rely on underlying session here, the logic has to be properly
implemented in Ignite's session implementation.

BTW, if you're working on the ticket, please assign it to yourself and
follow the process. More details here:
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute

-Val

On Wed, Jul 5, 2017 at 3:25 AM, yucigou <[hidden email]> wrote:

> Ignite WebSessionV2 uses genuineSes as the original HttpSession.
>
> Therefore, when setting an attribute or setting the maxInactiveInterval,
> Ignite should tell the original HttpSession about it.
>
> Otherwise, when the web container (such as Tomcat) thinks that a session
> expires, or is invalidated, or a session attribute gets removed, etc.,
> session attributes' HttpSessionBindingListener's valueUnbound callback
> function will not get fired.
>
> So once the original HttpSession gets updated with the session attributes
> and the maxInactiveInterval, the web container will transitively trigger
> the
> session attributes' HttpSessionBindingListener's valueUnbound callback
> function when a session expires, etc.
>
> (By the way, tested with our app, and our issue is fixed:
> https://github.com/apache/ignite/pull/2243)
>
>
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/It-seems-WebSession-s-
> removeAttribute-does-not-support-HttpSessionBindingListener-
> tp19184p19480.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>

[yucigou]

Hi Val,

The mechanism is similar to the implementation of invalidate() of the WebSessionV2 class. The {@link #invalidate()} action is delegated to the genuine session. Similarly, actions setAttribute(), removeAttribute(), and setMaxInactiveInterval() should be delegated to the genuine session. This way, the web container can do to the session whatever it promises to do, such as calling the HttpSessionBindingListener's valueUnbound callback function, etc.

If you look at the HttpSession interface, this is the whole list of APIs concerned:

* setAttribute()
* removeAttribute()
* setMaxInactiveInterval()
* invalidate()
* putValue()

And putValue() has been covered by setAttribute() in WebSessionV2

There are two main reasons that WebSessionV2 should delegate to the genuine session:
1. Avoid re-inventing the wheel. The web container has already implemented the related APIs.
2. WebSessionV2 is not visible to the web container. When the web container decides to expire the session, it will not reach the WebSessionV2 implementation. And this is exactly where I had the problem in the first place.

By the way, thanks for pointing out removing attributes, I've made another pull request on GitHub:
https://github.com/apache/ignite/pull/2243

Also I can't assign the ticket to myself because of lack of permission: https://issues.apache.org/jira/browse/IGNITE-5607

[Valentin Kulichenko]

This will not work. WebSessionV2 does not reinvent the wheel, it provides
additional functionality. In particular, allows to use the same session in
a clustered environment.

Genuine session is local, so if you just rely on it, all session data will
be lost when server that holds this session fails. Your listeners will not
be invoked as well, BTW. That's exactly what we're trying to avoid by
introducing this feature.

However, I agree that there is an issue with expiration. It's currently
handled based on ExpiryPolicy, i.e., if maxInactiveInterval is set, session
will be removed from the cache. But looks like we do not invalidate the
genuine session, which is wrong.

I think we should add a CacheEntryListener that will listen to expirations
and handle all required post actions - invalidation of genuine session and
invoking the listeners.

-Val

On Fri, Jul 7, 2017 at 6:59 AM, yucigou <[hidden email]> wrote:

> Hi Val,
>
> The mechanism is similar to the implementation of invalidate() of the
> WebSessionV2 class. The {@link #invalidate()} action is delegated to the
> genuine session. Similarly, actions setAttribute(), removeAttribute(), and
> setMaxInactiveInterval() should be delegated to the genuine session. This
> way, the web container can do to the session whatever it promises to do,
> such as calling the HttpSessionBindingListener's valueUnbound callback
> function, etc.
>
> If you look at the HttpSession interface, this is the whole list of APIs
> concerned:
>
> * setAttribute()
> * removeAttribute()
> * setMaxInactiveInterval()
> * invalidate()
> * putValue()
>
> And putValue() has been covered by setAttribute() in WebSessionV2
>
> There are two main reasons that WebSessionV2 should delegate to the genuine
> session:
> 1. Avoid re-inventing the wheel. The web container has already implemented
> the related APIs.
> 2. WebSessionV2 is not visible to the web container. When the web container
> decides to expire the session, it will not reach the WebSessionV2
> implementation. And this is exactly where I had the problem in the first
> place.
>
> By the way, thanks for pointing out removing attributes, I've made another
> pull request on GitHub:
> https://github.com/apache/ignite/pull/2243
>
> Also I can't assign the ticket to myself because of lack of permission:
> https://issues.apache.org/jira/browse/IGNITE-5607
>
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/It-seems-WebSession-s-
> removeAttribute-does-not-support-HttpSessionBindingListener-
> tp19184p19575.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>

[Valentin Kulichenko]

What is your Jira ID? I will add you as a contributor.

-Val

On Fri, Jul 7, 2017 at 2:44 PM, Valentin Kulichenko <
[hidden email]> wrote:

> This will not work. WebSessionV2 does not reinvent the wheel, it provides
> additional functionality. In particular, allows to use the same session in
> a clustered environment.
>
> Genuine session is local, so if you just rely on it, all session data will
> be lost when server that holds this session fails. Your listeners will not
> be invoked as well, BTW. That's exactly what we're trying to avoid by
> introducing this feature.
>
> However, I agree that there is an issue with expiration. It's currently
> handled based on ExpiryPolicy, i.e., if maxInactiveInterval is set, session
> will be removed from the cache. But looks like we do not invalidate the
> genuine session, which is wrong.
>
> I think we should add a CacheEntryListener that will listen to expirations
> and handle all required post actions - invalidation of genuine session and
> invoking the listeners.
>
> -Val
>
> On Fri, Jul 7, 2017 at 6:59 AM, yucigou <[hidden email]> wrote:
>
>> Hi Val,
>>
>> The mechanism is similar to the implementation of invalidate() of the
>> WebSessionV2 class. The {@link #invalidate()} action is delegated to the
>> genuine session. Similarly, actions setAttribute(), removeAttribute(), and
>> setMaxInactiveInterval() should be delegated to the genuine session. This
>> way, the web container can do to the session whatever it promises to do,
>> such as calling the HttpSessionBindingListener's valueUnbound callback
>> function, etc.
>>
>> If you look at the HttpSession interface, this is the whole list of APIs
>> concerned:
>>
>> * setAttribute()
>> * removeAttribute()
>> * setMaxInactiveInterval()
>> * invalidate()
>> * putValue()
>>
>> And putValue() has been covered by setAttribute() in WebSessionV2
>>
>> There are two main reasons that WebSessionV2 should delegate to the
>> genuine
>> session:
>> 1. Avoid re-inventing the wheel. The web container has already implemented
>> the related APIs.
>> 2. WebSessionV2 is not visible to the web container. When the web
>> container
>> decides to expire the session, it will not reach the WebSessionV2
>> implementation. And this is exactly where I had the problem in the first
>> place.
>>
>> By the way, thanks for pointing out removing attributes, I've made another
>> pull request on GitHub:
>> https://github.com/apache/ignite/pull/2243
>>
>> Also I can't assign the ticket to myself because of lack of permission:
>> https://issues.apache.org/jira/browse/IGNITE-5607
>>
>>
>>
>>
>> --
>> View this message in context: http://apache-ignite-developer
>> s.2346864.n4.nabble.com/It-seems-WebSession-s-removeAttri
>> bute-does-not-support-HttpSessionBindingListener-tp19184p19575.html
>> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>>
>
>

[yucigou]

Hi Val, I've registered two listeners with the WebSession cache and also the binary cache. When a session expires, the corresponding listeners will be invoked.

https://github.com/apache/ignite/pull/2243/files

Any comments? Thanks.

Yuci

[Valentin Kulichenko]

Hi Yuci,

I will review your changes this week.

-Val

On Mon, Jul 24, 2017 at 9:12 AM, yucigou <[hidden email]> wrote:

> Hi Val, I've registered two listeners with the WebSession cache and also
> the
> binary cache. When a session expires, the corresponding listeners will be
> invoked.
>
> https://github.com/apache/ignite/pull/2243/files
>
> Any comments? Thanks.
>
> Yuci
>
>
>
> --
> View this message in context: http://apache-ignite-
> developers.2346864.n4.nabble.com/It-seems-WebSession-s-
> removeAttribute-does-not-support-HttpSessionBindingListener-
> tp19184p19977.html
> Sent from the Apache Ignite Developers mailing list archive at Nabble.com.
>