38

UPDATE: Solution right after question.

Question:

Usually, synchronization is serializing parallel requests within a JVM, e.g.

private static final Object LOCK = new Object();

public void doSomething() {
  ...
  synchronized(LOCK) {
    ...
  }
  ...
}

When looking at web applications, some synchronization on "JVM global" scope is maybe becoming a performance bottleneck and synchronization only within the scope of the user's HttpSession would make more sense.

Is the following code a possibility? I doubt that synchronizing on the session object is a good idea but it would be interesting to hear your thoughts.

HttpSession session = getHttpServletRequest().getSession();
synchronized (session) {
  ...
}

Key Question:
Is it guaranteed that the session object is the same instance for all threads processing requests from the same user?

Summarized answer / solution:

It seems that the session object itself is not always the same as it depends on the implementation of the servlet container (Tomcat, Glassfish, ...) and the getSession() method might return just a wrapper instance.

So it is recommended to use a custom variable stored in the session to be used as locking object.

Here is my code proposal, feedback is welcome:

somewhere in a Helper Class, e.g. MyHelper:

private static final Object LOCK = new Object();

public static Object getSessionLock(HttpServletRequest request, String lockName) {
    if (lockName == null) lockName = "SESSION_LOCK";
    Object result = request.getSession().getAttribute(lockName);
    if (result == null) {
        // only if there is no session-lock object in the session we apply the global lock
        synchronized (LOCK) {
            // as it can be that another thread has updated the session-lock object in the meantime, we have to read it again from the session and create it only if it is not there yet!
            result = request.getSession().getAttribute(lockName);
            if (result == null) {
                result = new Object();
                request.getSession().setAttribute(lockName, result);
            }
        }
    }
    return result;
}

and then you can use it:

Object sessionLock = MyHelper.getSessionLock(getRequest(), null);
synchronized (sessionLock) {
  ...
}

Any comments on this solution?

Community
  • 1
  • 1
basZero
  • 4,129
  • 9
  • 51
  • 89
  • possible duplicate of [Is HttpSession thread safe, are set/get Attribute thread safe operations?](http://stackoverflow.com/questions/616601/is-httpsession-thread-safe-are-set-get-attribute-thread-safe-operations) – Sahil Muthoo Mar 21 '12 at 10:08
  • 3
    It is not a duplicate as the linked question is about thread safety of session variables. I am asking whether you can lock on the session object itself. – basZero Mar 21 '12 at 10:14
  • 2
    @SahilMuthoo: I disagree, the OP asks which object is safe to be used for `HttpSession` synchronization/locking, not whether `HttpSession` is thread-safe or not. – Tomasz Nurkiewicz Mar 21 '12 at 10:14
  • 3
    In doubt you could put a property in your session just for locking. Be aware though, that `synchronized` will not work if you use a cluster with session replication and when several requests for the same session are handled by different servers. – user1252434 Mar 21 '12 at 10:27
  • @user1252434 like the code snippet in my question? I updated it with a possible solution. – basZero Mar 21 '12 at 12:44
  • Yes, that's what I had in mind. With less locking to add the property, though. :) Depending on your web framework you could try to put the lock object in the session context immediately after session creation. – user1252434 Mar 21 '12 at 13:49
  • @user1252434 Yes, thought about that too, but this way, if the lock object is available in the session, the global lock is not used anymore, so there should not be that much performance loss in this case (but the code looks ugly, I know). – basZero Mar 26 '12 at 07:29
  • The global LOCK object is necessary in your case (otherwise two requests from the same user with the same session might create different SESSION_LOCK objects), and imo, it is going to create some performance loss since all the requests (even from different users) will be serialized. – despot Apr 29 '14 at 11:12
  • The best way (imo), as described in the accepted answer, is using the HttpSessionMutexListener in combination with WebUtils.getSessionMutex(session) to get the appropriate lock. In this case the only issue might be if two requests without session from the same user try to create a session (I cannot see Spring using a global lock for this) which should happen with a smaller probability than with your case. – despot Apr 29 '14 at 11:15
  • 1
    @basZero - I like your propsed solution; it is robust and provides for multiple session locks for different purposes (a very nice idea). But instead of appending it to your question, you should add it as an answer below in its own right; that way I can up-vote your answer as well as your question. :-) – daiscog Jul 20 '15 at 12:06
  • 1
    I did something similar :) – brat Sep 03 '21 at 13:51
  • @daiscog: just did that :) – basZero Sep 22 '21 at 12:56

10 Answers10

36

I found this nice explanation in JavaDoc for WebUtils.getSessionMutex():

In many cases, the HttpSession reference itself is a safe mutex as well, since it will always be the same object reference for the same active logical session. However, this is not guaranteed across different servlet containers; the only 100% safe way is a session mutex.

This method is used as a lock when synchronizeOnSession flag is set:

Object mutex = WebUtils.getSessionMutex(session);
synchronized (mutex) {
    return handleRequestInternal(request, response);
}

If you look at the implementation of getSessionMutex(), it actually uses some custom session attribute if present (under org.springframework.web.util.WebUtils.MUTEX key) or HttpSession instance if not:

Object mutex = session.getAttribute(SESSION_MUTEX_ATTRIBUTE);
if (mutex == null) {
    mutex = session;
}
return mutex;

Back to plain servlet spec - to be 100% sure use custom session attribute rather than HttpSession object itself.

See also

Tomasz Nurkiewicz
  • 334,321
  • 69
  • 703
  • 674
  • 1
    @basZero: no, I have shown code samples from Spring MVC source just to give you an example how it is implemented in mature web framework. You can base your own solution on these simple concepts. – Tomasz Nurkiewicz Mar 21 '12 at 12:21
  • @Tomasz_Nurkiewicz ok, updated my question with a custom solution proposal. – basZero Mar 21 '12 at 12:53
  • 3
    This omits an important moment: thread-safely creating the mutex itself. The best approach probably is to do it in a `HttpSessionListener` on a `sessionCreated` event. – Vsevolod Golovanov Oct 05 '18 at 13:01
9

In general, don't rely on HttpServletRequest.getSession() returning same object. It's easy for servlet filters to create a wrapper around session for whatever reason. Your code will only see this wrapper, and it will be different object on each request. Put some shared lock into the session itself. (Too bad there is no putIfAbsent though).

Peter Štibraný
  • 32,463
  • 16
  • 90
  • 116
  • Very good point regarding wrappers. The idea to put an object into the session is very good. Did you use that already? – basZero Mar 21 '12 at 10:31
  • I've used combined technique ... lock object stored in the session, and synchronizing on the session itself for putting this lock object into it :-) – Peter Štibraný Mar 21 '12 at 10:34
  • Hi @PeterŠtibraný , something like in my summarized answer above? – basZero Mar 21 '12 at 12:09
  • 2
    @basZero: yes, your code is almost line-to-line identical to mine. I've done the synchronization block on the session itself, but your solution is better (exactly for the reason stated in my answer). – Peter Štibraný Mar 21 '12 at 13:21
3

As people already said, sessions can be wrapped by the servlet containers and this generates a problem: the session hashCode() is different between requests, i.e., they are not the same instance and thus can't be synchronized! Many containers allow persist a session. In this cases, in certain time, when session was expired, it is persisted on disk. Even when session is retrieved by deserialization, it is not same object as earlier, because it don't shares same memory address like when was at memory before the serialization process. When session is loaded from disk, it is put into memory for further access, until "maxInactiveInterval" is reached (expires). Summing up: the session could be not the same between many web requests! It will be the same while is in memory. Even if you put an attribute into the session to share lock, it will not work, because it will be serialized as well in the persistence phase.

Lucas Batistussi
  • 2,283
  • 3
  • 27
  • 35
  • So you are saying that my documented solution won't work? I thought synchronizing on an object in the session (attribute) will work. – basZero Oct 27 '13 at 17:53
  • No not yet, I'm in the process of finding the correct solution before I start implementation – basZero Oct 28 '13 at 19:54
2

Here is my own solution:

It seems that the session object itself is not always the same as it depends on the implementation of the servlet container (Tomcat, Glassfish, ...) and the getSession() method might return just a wrapper instance.

So it is recommended to use a custom variable stored in the session to be used as locking object.

Here is my code proposal, feedback is welcome:

somewhere in a Helper Class, e.g. MyHelper:

private static final Object LOCK = new Object();

public static Object getSessionLock(HttpServletRequest request, String lockName) {
    if (lockName == null) lockName = "SESSION_LOCK";
    Object result = request.getSession().getAttribute(lockName);
    if (result == null) {
        // only if there is no session-lock object in the session we apply the global lock
        synchronized (LOCK) {
            // as it can be that another thread has updated the session-lock object in the meantime, we have to read it again from the session and create it only if it is not there yet!
            result = request.getSession().getAttribute(lockName);
            if (result == null) {
                result = new Object();
                request.getSession().setAttribute(lockName, result);
            }
        }
    }
    return result;
}

and then you can use it:

Object sessionLock = MyHelper.getSessionLock(getRequest(), null);
synchronized (sessionLock) {
  ...
}

Any comments on this solution?

basZero
  • 4,129
  • 9
  • 51
  • 89
  • 2
    it seems to me this has similar issue as [double-locking](https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html): you check if a reference to an object is `null` outside of synchronized block, so you may get `false` (non-null, that is), but the object may not be fully initialized yet due to race conditions and write reorderings by compiler/VM. – morgwai Nov 08 '21 at 10:53
  • 1
    A better approach probably is to add the lock object for a session directly when the session is created. This can be done in a HttpSessionListener on a sessionCreated event. – mihca Sep 21 '22 at 07:44
  • Double-checked locking requires a volatile variable to guarantee that reads and writes are not cached and not reordered. I'm not sure setAttribute and getAttribute guarantee the same properties, but this article seems to say that they guarantee visibility indeed http://web.archive.org/web/20210109101727/https://www.ibm.com/developerworks/library/j-jtp09238/index.html Any comments on that? – marcus Mar 30 '23 at 20:18
2

Synchronization occurs when a lock is placed on an object reference, so that threads that reference the same object will treat any synchronization on that shared object as a toll gate.

So, what your question raises an interesting point: Does the HttpSession object in two separate web calls from the same session end up as the same object reference in the web container, or are they two objects that just happen to have similar data in them? I found this interesting discussion on stateful web apps which discusses HttpSession somewhat. Also, there is this discussion at CodeRanch about thread safety in HttpSession.

From those discussions, it seems like the HttpSession is indeed the same object. One easy test would be to write a simple servlet, look at the HttpServletRequest.getSession(), and see if it references the same session object on multiple calls. If it does, then I think your theory is sound and you could use it to sync between user calls.

CodeChimp
  • 8,016
  • 5
  • 41
  • 79
  • Thanks a lot for the [first pointer](http://www.ibm.com/developerworks/library/j-jtp09238/index.html), the author Brian Goetz is anyway the Guru when it comes to concurrency. As of his writing, it seems to be OK to synchronize on the session object: _Serializing requests on an HttpSession makes many concurrency hazards go away_ – basZero Mar 21 '12 at 10:25
  • 1
    There are two problems, though: 1. The Servlet spec doesn't say that the return value of getSession should always be the same object. 2. If you have a filter (from a 3rd party lib, or otherwise) which wraps the Request object, that might also wrap the return value of getSession with a new wrapper every time. In short, if there are no guarantees, it is not safe. – daiscog Aug 18 '15 at 11:41
  • Man, @daiscog, talk about a blast-from-the-past...this was from like 3yrs ago! Anyway, I agree with you point and would reprimand my younger self for not thinking of all sides of the issue. – CodeChimp Aug 18 '15 at 21:29
1

Another solution suggested in "Murach's Java Servlets and JSP (3rd Edition)" book:

Cart cart;
final Object lock = request.getSession().getId().intern();
synchronized (lock) {
    cart = (Cart) session.getAttribute("cart");
}
Max77
  • 1,466
  • 13
  • 19
  • 2
    interesting idea, however this serializes ALL requests of a user whereas my proposed solution can handle different user specific locks so that it is up to you to decide on serialization of user reqeusts. – basZero Jul 08 '15 at 08:13
  • Never intern string with default String.intern(). Otherwise you will flood String internal map with your own values without any possibility to clean it. Create your own interner, or better use libraries like guava or others. – Denis Kokorin Dec 03 '18 at 14:35
  • @DenisKokorin That is no longer true with Java 7 and above. https://www.baeldung.com/java-string-pool#garbage-collection – Number945 Jan 03 '21 at 13:39
  • I couldn't find any reliable information if `intern()` is thread-safe if 2 separate `ClassLoader`s load `String` class (also not entirely sure if it is possible to happen for a `java.lang` class to be loaded by 2 separate `ClassLoaders`...) – morgwai Nov 08 '21 at 05:57
1

Personally, I implement session-locking with the help of an HttpSessionListener*:

package com.example;

@WebListener
public final class SessionMutex implements HttpSessionListener {
    /**
     * HttpSession attribute name for the session mutex object.  The target for 
     * this attribute in an HttpSession should never be altered after creation!
     */
    private static final String SESSION_MUTEX = "com.example.SessionMutex.SESSION_MUTEX";

    public static Object getMutex(HttpSession session) {
        // NOTE:  We cannot create the mutex object if it is absent from  
        // the session in this method without locking on a global 
        // constant, as two concurrent calls to this method may then 
        // return two different objects!  
        //
        // To avoid having to lock on a global even just once, the mutex 
        // object is instead created when the session is created in the 
        // sessionCreated method, below.

        Object mutex = session.getAttribute(SESSION_MUTEX);

        // A paranoia check here to ensure we never return a non-null 
        // value.  Theoretically, SESSION_MUTEX should always be set, 
        // but some evil external code might unset it:
        if (mutex == null) {
            // sync on a constant to protect against concurrent calls to 
            // this method
            synchronized (SESSION_MUTEX) { 
                // mutex might have since been set in another thread 
                // whilst this one was waiting for sync on SESSION_MUTEX
                // so double-check it is still null:
                mutex = session.getAttribute(SESSION_MUTEX);
                if (mutex == null) {
                    mutex = new Object();
                    session.setAttribute(SESSION_MUTEX, mutex);
                }
            }
        }
        return mutex; 
    }

    @Override
    public void sessionCreated(HttpSessionEvent hse) {
        hse.getSession().setAttribute(SESSION_MUTEX, new Object());
    }

    @Override
    public void sessionDestroyed(HttpSessionEvent hse) {
        // no-op
    }
}

When I need a session mutex, I can then use:

synchronized (SessionMutex.getMutex(request.getSession())) {
    // ...
}

__

*FWIW, I really like the solution proposed in the question itself, as it provides for named session locks so that requests for independent resources don't need to share the same session lock. But if a single session lock is what you want, then this answer might be right up your street.

daiscog
  • 11,441
  • 6
  • 50
  • 62
  • 1
    Double check-lock in your implementation is wrong. One can not simply read without locks/synchronization data written with locks/synchronization. Check this please https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html – Denis Kokorin Dec 03 '18 at 14:38
0

The spring framework solution as mentioned by Tomasz Nurkiewicz is accidentally correct in clustered environments only because the Servlet spec requires session consistency across multiple JVMs. Otherwise, it does not do a magic on its own for the scenarios where multiple requests are spread across different machines. See the discussion in this thread that sheds some light on the subject.

Community
  • 1
  • 1
  • the OP is about "_serializing parallel requests **within a JVM**_". There's no way a `synchronized` block has anything to do with synchronization across separate JVMs or machines. For this, a distributed consensus mechanism would need to be used. – morgwai Nov 08 '21 at 13:57
0

Using

private static final Object LOCK = new Object();

you are using the same lock for all sessions and it was the core reason for deadlock I did face. So every session in your implementation has the same race condition, which is bad.

It needs change.

Other suggested answer:

Object mutex = session.getAttribute(SESSION_MUTEX_ATTRIBUTE);
if (mutex == null) {
  mutex = session;
}
return mutex;

seems much better.

Mladen Adamovic
  • 3,071
  • 2
  • 29
  • 44
  • 1
    Thanks for your reply, but have a look at my summarized solution at the top. The object `LOCK` is only used when the session attributed is created, this is required to synchronize globally in order to avoid to requests creating objects in the session. – basZero Oct 30 '13 at 09:53
  • I got deadlocks at my webapp numbeo.com when trying to synchronize globally when the session attribute is created. The underlying reason might be that some request, session and response methods might require round trip time which could go up to 1s max latency time. – Mladen Adamovic Oct 31 '13 at 07:50
  • 1
    synchronizing globally is a valid use case. your deadlocks might come from your implementation, how you use them. – basZero Nov 01 '13 at 08:53
  • are you sure that request, response and session method invoke after all request has been read in your web server (so it is cached in the container). If so, than you are right. – Mladen Adamovic Nov 02 '13 at 10:01
0

The answers are correct. If you want to avoid the same user executes 2 different (or the same) requests at the same time, you can synchronize on the HttpSession. The best to do this is to use a Filter.

Notes:

  • if your resources (images, scripts, and any non-dynamic file) also comes through the servlet, you could create a bottleneck. Then be sure, the synchonization is only done on dynamic pages.
  • Try to avoid the getSession directly, you should better test if the session already exists because a session is not automatically created for guests (as nothing has to be stored in the session). Then, if you call getSession(), the session will be created and memory will be lost. Then use getSession(false) and try to deal with the null result if no session already exists (in this case, don't synchronize).
William R
  • 185
  • 1
  • 10
  • 1
    If you do synchronization inside filter, then you're essentially disabling concurrent request processing for same session. It may or may not be what you want, but it sounds like too broad synchronization to me. One should always strive to synchronize on smallest possible area. – Peter Štibraný Mar 21 '12 at 10:28
  • Thanks for pointing out not to create useless session objects. The section where I would synchronize on the session is only within one specific usecase where the user is anyway logged in. – basZero Mar 21 '12 at 10:29
  • as explained in other answers, sync on session is not safe due to possible wrappings – morgwai Nov 08 '21 at 11:01