-->
These old forums are deprecated now and set to read-only. We are waiting for you on our new forums!
More modern, Discourse-based and with GitHub/Google/Twitter authentication built-in.

All times are UTC - 5 hours [ DST ]



Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 8 posts ] 
Author Message
 Post subject: synchronizing HTTPSession - don't do it
PostPosted: Thu Sep 30, 2004 8:25 am 
Beginner
Beginner

Joined: Thu Sep 11, 2003 12:53 pm
Posts: 23
Hi,

Just finished Hibernate in Action. Great book - thanks!

One minor nit. On page 328 the book recommends avoiding the problem of form double-submission by serializing the HTTPSession object.

Actually, this doesn't work in all containers. Since HTTPSession is an interface facade not an object this might be a different object for different requests. Serialize on an object in the session instead.

I've explicitly found this to be required in Tomcat 4.1.

a reference:
http://www.mail-archive.com/tomcat-user ... 08029.html

WILL


Top
 Profile  
 
 Post subject:
PostPosted: Thu Mar 03, 2005 4:43 am 
Newbie

Joined: Wed Mar 02, 2005 1:28 pm
Posts: 18
I also think that this should be corrected in the book and put in the errata.

The assumption made by the authors that there is a single HttpSession instance for one sessionid has no support in the J2EE Servlet Specification. The container is free to use several concurrent HttpSession instances for one "user session", even in the same webapp and JVM. Thus, there is no single HttpSession instance to use for synchronization.
The rules for session attributes are stricter, so typically you would create one of these to sync on instead, just as wglass mentions.
BTW, BEA Weblogic also exhibits this "multiple HttpSession instances" behaviour.

Even though this book is not about learning webapp programming I think it is unfortunate that ill advice like this has not been edited out, as the "sync on HttpSession" pattern introduces extremely subtle and hard-to-find concurrency bugs.


Top
 Profile  
 
 Post subject:
PostPosted: Thu Mar 03, 2005 9:42 am 
Hibernate Team
Hibernate Team

Joined: Mon Aug 25, 2003 9:11 pm
Posts: 4592
Location: Switzerland
I apologize for not writing a perfect book. I will add it to the errata if I've time.


Top
 Profile  
 
 Post subject:
PostPosted: Thu Mar 03, 2005 10:54 am 
Beginner
Beginner

Joined: Thu Sep 11, 2003 12:53 pm
Posts: 23
Ha! Christian, this is one of the best tech books I've read this past year. I gave a copy as a Christmas present to each member of my team.

Thanks for addressing this extremely minor point.


Top
 Profile  
 
 Post subject:
PostPosted: Wed Mar 30, 2005 12:13 am 
Beginner
Beginner

Joined: Wed Dec 03, 2003 10:59 am
Posts: 47
Hello,

wglass & mikewse - I just posted a very related question here:
http://forum.hibernate.org/viewtopic.php?t=940560

Since Hibernate Session is not thread-safe, and a user can make several requests to a webapp in parallel, how do people typically handle these parallel requests? How do you do it? Obviously, you don't synchronize on HttpSession :) Do you synchronize on the Hibernate Session? In the servlet filter or?

Thanks,
Otis


Top
 Profile  
 
 Post subject:
PostPosted: Wed Mar 30, 2005 12:47 am 
Beginner
Beginner

Joined: Thu Sep 11, 2003 12:53 pm
Posts: 23
Use a homemade mutex object. In your code, synchronize as follows:

Code:
synchronized (Util.getSessionMutex()) {
   // code to synchronize goes here
}


then write a utility class with these methods

Code:
/**
  * Retrieve an object guaranteed to be the same across a session.  Use for synchronizing.
  * @return
  */
public Object getSessionMutex()
{
   // no session? assume this is being run for testing purposes and there is no
   // simultaneous sessions
   if (getRequest() == null)
      return Boolean.TRUE;

   else {
      HttpSession session = getRequest().getSession();
      Object ret = session.getAttribute(SESSION_MUTEX_ATTRIBUTE);
      if (ret == null) ret = createSessionMutex(session);
     return ret;
   }
  }

/**
* Use to create a new mutex.
* @author wglass
*/
private synchronized Object createSessionMutex(HttpSession session)
{
     // check again it doesn't already exist
     if (session.getAttribute(SESSION_MUTEX_ATTRIBUTE) == null) {
        Object o = new Object();
        session.setAttribute(SESSION_MUTEX_ATTRIBUTE, o);
        return o;

     } else
        return session.getAttribute(SESSION_MUTEX_ATTRIBUTE);
}



Note that this could be criticized on the grounds of containing the antipattern "double-checked locking" except that the mutex is an immutable object.


Top
 Profile  
 
 Post subject:
PostPosted: Wed Feb 22, 2006 5:39 am 
Newbie

Joined: Wed Mar 02, 2005 1:28 pm
Posts: 18
[A late followup - where did those reply notifications go...? ;-)]

wglass wrote:
Code:
   // no session? assume this is being run for testing purposes and there is no
   // simultaneous sessions
   if (getRequest() == null)
      return Boolean.TRUE;

   else {
      HttpSession session = getRequest().getSession();


Do you mean to use the "testing purposes" branch when there is no request or no session? (The comment and code are sort of contradictory.)

If it is for the case of no request all is fine as this branch would never trigger during normal webapp runtime. But if you actually mean to test for no session, then you have a Denial of Service vulnerability in this code as this branch would trigger for all requests from new clients that haven't yet had a session created for them.

The Denial of Service problem is that those clients will all be synchronized on the same object (the shared constant object Boolean.TRUE) and only one of them can execute at a time. If one of the clients for some reason stops communicating with your server (there are bugs in IE that can cause this) while holding the lock, all other new clients will have to wait for your server to timeout the hanging connection. Depending on your server, this timeout may be set to anything between 30 secs and infinite.

Also, be aware that the call
HttpSession session = getRequest().getSession();
will create the session if it doesn't exist. If this is what you want all is fine, but if you have code paths in your server that do not rely on the session, and you don't want to create unnecessary sessions, consider using getSession(false) instead.

BTW: I like the dual-check thingie. It's nice to wait with acquiring the container-wide lock until it's absolutely necessary!


Top
 Profile  
 
 Post subject:
PostPosted: Wed Feb 22, 2006 11:28 am 
Beginner
Beginner

Joined: Thu Sep 11, 2003 12:53 pm
Posts: 23
Hi,

You can just ignore the null check for getRequest(). The code excerpt was from a recent project that is sometimes run by test code outside of a servlet. In such cases there is no request object. Since the test code is running in single thread mode there is also no need to get a true session mutex.

WILL


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 8 posts ] 

All times are UTC - 5 hours [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
© Copyright 2014, Red Hat Inc. All rights reserved. JBoss and Hibernate are registered trademarks and servicemarks of Red Hat, Inc.