cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nathaniel Alfred" <Alfred.Nathan...@swx.com>
Subject RE: Synchronization on session object (was svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java)
Date Fri, 13 May 2005 09:37:48 GMT
> -----Original Message-----
> From: Ralph Goers [mailto:Ralph.Goers@dslextreme.com]
> Sent: Freitag, 13. Mai 2005 08:57
> To: dev@cocoon.apache.org
> Subject: Re: svn commit: r169856 - /cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/http/HttpRequest.java

> >
> >You don't want to replace the outer synchronized(serverSession) by 
> >synchronized(sessions) because that blocks all threads without being
> >necessary (although the effect will be immeasurable).
> >  
> >
> Yes, I do. The alternative is to synchronize on the servlet container's 
> session object, which could have far more horrifying results. Do you 
> have any idea how that will impact the container?  I don't because I 
> cannot know.  For all I know it is conceivable that doing that could 
> cause a deadlock in some wierd scenario.  I also fail to see how locking 
> the map causes any more of a performance hit than locking the session. 
> Since the map is only used by this one method its effect should be far 
> less of an impact that locking the session.  Besides, you ARE blocking 
> all threads on the get and put anyway, since it has been declared a 
> synchronized hash map. In fact, it is being done twice inside of a 
> synchronized block.

Global synchronization on sessions saves two lock operations and gives 
better single-threaded performance.

Local synchronization on serverSession/sessions.get/sessions.put gives
better multi-threaded concurrency.

Both effects are really minute.  I now tend to favour your proposal to
use the global lock because is saves a lot of brain cycles during code
inspection.

However, I am amazed by your categoric opposition to locking the
serverSession.  The whole story started because Joerg wants to use the 
session object in order to coordinate concurrent requests belonging to 
the same session.  I had a difference of opinion with him as well about
the object identity guarantees in HttpRequest.getSession.

I now read it up in the Servlet specs.  Both 2.3 and 2.4 use the same
wording in SRV.7.7.1 Threading Issues:

  Multiple servlets executing request threads may have active access
  to a single session object at the same time.  The Developer has the
  responsability for synchronizing access to session resources as
  appropriate.

That clearly states that synchronized(serverSession) is allowed and
must be used when necessary.

It does not settle my dispute with Joerg though.  One may read the
first sentence as

  "Concurrent requests for the same session may happen and they
   must all be given the same session object." (Joerg)

or as 

  "Concurrent requests for the same session *may* (but need not)
   be given the same session object." (Alfred)

I think we agree that during the lifetime of a session it is not
necessarily represented always by the same Java object.  A clever
container may move it to another cluster node or backing store,
and can hardly be expected to restore it into the same object.

If there is no guarantee that the session object stays the same
between sequential requests, why should there be such a guarantee
for concurrent requests?  Even if the people doing the specs intended
to provide that guarantee, there are still the implementators to
read it the same way as I do or to mess it up.

For example, in Tomcat's PersistenceManager I can't see any protection 
against two requests racing in swapIn and restoring the same session
into two different Java objects.

So it is a shakey assumption that the session object returned from
the container can be used to coordinate concurrent threads.  It works
in normal environments but there is a small chance that it can fail
for complex environments.  I wouldn't bet my head on it.

Now you may argue that Joerg is not using the container session but
the Cocoon wrapper for which the hashmap guarantees that it is always
the same Java object for the same session.

Well, no, not really.  It depends on how equals() is implemented by 
the container session object.  If it does string compares of the session
ids it is fine.  

If it inherits Object.equals, then you loose because the current version 
will produce a new wrapper for every session object.  Since normally one
does not need to compare session objects for equality I doubt that 
container implementers usually bother to override equals and hashCode.

To be safe one should use the sessions.put(serverSession.getId(), session)
but then I don't know anymore how to use weak references for solving the
memory leak problem.  And does the container react if the request uses
a different session object than intended even if it represents the same
session.

Bottomline:

I think synchronized(session) should never be used as vehicle to 
coordinate concurrent requests because there is no convincing guarantee 
that it is always working as expected.  

Joerg, if you want to do it in your usercode, I don't mind, but please
don't use it in common Cocoon code.  My propesed alternative of
synchronized(session.getId().intern()) may look obscure but at least
it is guaranteed to work.

In my interpretation the old getSession version was already compliant to
Servlet specs.  Whether to keep the new version I am +0.  It avoids a
programming error to manifest in simple environments but in complex
setups it just may shift the error rate from 1/thousand to 1/million -
and this is really one of the worst situations.

But maybe I am just paranoid?

Anybody who knows an authoritative statement on the isolation level
for session attributes?

Cheers, Alfred.
 
 
This message is for the named person's use only. It may contain confidential, proprietary
or legally privileged information. No confidentiality or privilege is waived or lost by any
mistransmission. If you receive this message in error, please notify the sender urgently and
then immediately delete the message and any copies of it from your system. Please also immediately
destroy any hardcopies of the message. You must not, directly or indirectly, use, disclose,
distribute, print, or copy any part of this message if you are not the intended recipient.
The sender’s company reserves the right to monitor all e-mail communications through their
networks. Any views expressed in this message are those of the individual sender, except where
the message states otherwise and the sender is authorised to state them to be the views of
the sender’s company.

Mime
View raw message