Would Like to request change to DefaultScriptSession.invalidate method

[Roger Desroches]

Hi,

   I'd like to change the order of the calls in the script session invalidate so that the sessions invalidate isn't set to true until the script session manager has a chance to do invalidate work.

  I have a registered ScriptSessionManager of my own that is trying to do work on some stores, but I can't get the attributes out of the ScriptSession because it's invalidated.  therefore, I'd like to set the invalidated flag AFTER the manager is done working with it.


    public void invalidate()
    {
        synchronized (invalidLock)
        {
            // the following two lines are swapped compared to current code
            manager.invalidate(this);
            invalidated = true;
        }
    }


What do you think?

roger

[Roger Desroches]

Oh blast it, never mind.  It's still invalidating itself base on the time when getAttribute is called.  I guess I'm just going to create another level of indirection and store all my attributes in another store hashed by session id.

sorry for the wasted bandwidth.
Roger

Roger Desroches wrote:

Hi,

   I'd like to change the order of the calls in the script session invalidate so that the sessions invalidate isn't set to true until the script session manager has a chance to do invalidate work.

  I have a registered ScriptSessionManager of my own that is trying to do work on some stores, but I can't get the attributes out of the ScriptSession because it's invalidated.  therefore, I'd like to set the invalidated flag AFTER the manager is done working with it.


    public void invalidate()
    {
        synchronized (invalidLock)
        {
            // the following two lines are swapped compared to current code
            manager.invalidate(this);
            invalidated = true;
        }
    }


What do you think?

roger

[tpeierls]

It wasn't a waste. I found a concurrency bug while looking at it. :-/

The invalidated field is @GuardedBy("invalidLock"), so all access to the field must be done with the lock held. The invalidateIfNeeded() method, however, tests the field outside of any synchronization.

Quickest fix would be to make the field volatile.

--tim

Roger Desroches wrote:

Oh blast it, never mind.  It's still invalidating itself base on the time when getAttribute is called.  I guess I'm just going to create another level of indirection and store all my attributes in another store hashed by session id.

sorry for the wasted bandwidth.
Roger

[Joe Walker-3]

Tim - you scare me! How the *%@£$ did you spot that?
Thanks.

Joe.

On Wed, Apr 9, 2008 at 3:08 PM, tpeierls <tim@...> wrote:

It wasn't a waste. I found a concurrency bug while looking at it. :-/

The invalidated field is @GuardedBy("invalidLock"), so all access to the
field must be done with the lock held. The invalidateIfNeeded() method,
however, tests the field outside of any synchronization.

Quickest fix would be to make the field volatile.

--tim

Roger Desroches wrote:
>
> Oh blast it, never mind.  It's still invalidating itself base on the time
> when getAttribute is called.  I guess I'm just going to create another
> level of indirection and store all my attributes in another store hashed
> by session id.
>
> sorry for the wasted bandwidth.
> Roger
>

--
View this message in context: http://www.nabble.com/Would-Like-to-request-change-to-DefaultScriptSession.invalidate-method-tp16585714p16585986.html

Sent from the DWR - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[tpeierls]

*sigh*

Ideally, we would use the real @GuardedBy annotation instead of just a comment, and then FindBugs could tell us whenever a guarded field was accessed without the corresponding lock being held.

Until then, we have to rely on lucky breaks and more (and better prepared) eyeballs.

--tim

Joe Walker-3 wrote:

Tim - you scare me! How the *%@£$ did you spot that?
Thanks.

Joe.

On Wed, Apr 9, 2008 at 3:08 PM, tpeierls <[e-mail]> wrote:

>
> It wasn't a waste. I found a concurrency bug while looking at it. :-/
>
> The invalidated field is @GuardedBy("invalidLock"), so all access to the
> field must be done with the lock held. The invalidateIfNeeded() method,
> however, tests the field outside of any synchronization.
>
> Quickest fix would be to make the field volatile.