Ouch.  Just looked at the
AbstractSessionManager<http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.jetty/jetty-server/9.1.0.v20131115/org/eclipse/jetty/server/session/AbstractSessionManager.java#AbstractSessionManager>...
Such synchronisation is the tip of the iceberg.

I wonder if there really needs to be this amount of synchronisation when
the order of the requests coming in that might affect a session can be
quote arbitrary in themselves.   On AbstractSessionManager synchronisation
protection on _attributes is perhaps overkill when a ConcurrentHashMap
could be used instead.





On 27 February 2014 20:02, "" <[email protected]> wrote:

> I don't believe the synchronised block is useful here.   Synchronised
> should be used to stop things breaking, not to protect against something
> that is occasionally inefficient.
>
> So this is sufficient and will avoid inconsistent synchronization
> warnings...
>
>    @Override
>    public void removeAttribute(String name) {
>
>
>       if(getAttribute(name)){
>          super.removeAttribute(name);
>          _dirty = true;
>       }
>
>    }
>
>
>
>
> On 27 February 2014 10:40, Jan Bartel <[email protected]> wrote:
>
>> Cemo,
>>
>> This will need some refactoring of the implementation of the session
>> interface, but I'm not opposed to looking into it.
>>
>> Can you open a bug for this please and assign to me?
>>
>> thanks
>> Jan
>>
>> On 27 February 2014 06:25, Cemo <[email protected]> wrote:
>> > Hi,
>> >
>> > Current org.eclipse.jetty.server.session.JDBCSessionManager has
>> > removeAttribute as this:
>> >
>> >   public void removeAttribute (String name)
>> >
>> >  {
>> >       super.removeAttribute(name);
>> >
>> >
>> >
>> >      _dirty=true;
>> >  }
>> >
>> >
>> > This has a side effect because of making dirty for every removing
>> operation.
>> > This is causing a lot of trouble because each jstl set ( c:set ) call
>> in JSP
>> > pages is also calling removeAttribute internally. What I am suggesting
>> is
>> > that something like this:
>> >
>> >
>> >    @Override
>> >    public void removeAttribute(String name) {
>> >
>> >
>> >
>> >       synchronized (this){
>> >          Object attribute = getAttribute(name);
>> >
>> >
>> >
>> >          if(attribute != null){
>> >
>> >
>> >
>> >             super.removeAttribute(name);
>> >
>> >
>> >
>> >             _dirty = true;
>> >          }
>> >       }
>> >    }
>> >
>> > public
>> >
>> >
>> > https://gist.github.com/cemo/9236abe34d2b126242ad
>> >
>> > What do you think?
>> >
>> > _______________________________________________
>> > jetty-users mailing list
>> > [email protected]
>> > https://dev.eclipse.org/mailman/listinfo/jetty-users
>> >
>>
>>
>>
>> --
>> Jan Bartel <[email protected]>
>> www.webtide.com
>> 'Expert Jetty/CometD developer,production,operations advice'
>> _______________________________________________
>> jetty-users mailing list
>> [email protected]
>> https://dev.eclipse.org/mailman/listinfo/jetty-users
>>
>
>
_______________________________________________
jetty-users mailing list
[email protected]
https://dev.eclipse.org/mailman/listinfo/jetty-users

Reply via email to