On Thu, 13 Jun 2002, Noah Levitt wrote:

> Date: Thu, 13 Jun 2002 14:45:35 -0400
> From: Noah Levitt <[EMAIL PROTECTED]>
> Reply-To: Struts Users Mailing List <[EMAIL PROTECTED]>
> To: Struts Users Mailing List <[EMAIL PROTECTED]>
> Subject: Re: thread safety
>
> Hello,
>
> Your suggestion is duly noted, and would probably be good
> advice for many web apps. But keeping form beans in session
> scope can be very handy at times, as has been pointed out in
> other posts. Sessions are one of the main advantages of
> servlets, after all.
>

Everything in life is a tradeoff ... session-scoped form beans survive in
between requests, but you have to worry about thread safety.  Request
scoped attributes don't survive, but thread safety is assured.  Pick the
approach that best meets your particular requirements, and deal with the
downside appropriately.

> So, you concede that using form beans the way they are
> commonly used, in session scope with setters and getters
> unsynchronized, is not thread-safe?
>

Not always.  Example -- let's say you have a String property (very common
in a form bean), so you have a setter like:

  private String foo = null;

  public String getFoo() {
    return (this.foo);
  }

  public void setFoo(String foo) {
    this.foo = foo;
  }

These methods do not need to be synchronized, because the assignments are
atomic.

The times you need to synchronize property setters is when they modify
some complex data structure (like a HashMap) that is not already ensuring
thread safety (the way Hashtable does).

NOTE 1:  If you have to synchronize the setter, you probably also have to
synchronize the getter so that it doesn't try to run when the setter is in
the middle of modifying the data structure.

NOTE 2:  It's better to synchronize on just the object being updated,
rather than adding the "synchronized" modifier to the method itself.  For
example, this:

  private HashMap foos = new HashMap();

  public synchronized Foo getFoo(String key) {
    return ((Foo) foos.get(key));
  }

  public synchronized void addFoo(Foo foo) {
    foos.put(foo.getKey(), foo);
  }

is essentially equivalent to this:

  private HashMap foos = new HashMap();

  public Foo getFoo(String key) {
    synchronized (this) {
      return ((Foo) foos.get(key));
    }
  }

  public void addFoo(Foo foo) {
    synchronized (this) {
      foos.put(foo.getKey(), foo);
    }
  }

which blocks other threads accessing *any* synchronized method on this
bean, even if they aren't touching the "foos" instance variable.  A better
approach:

  private HashMap foos = new HashMap();

  public Foo getFoo(String key) {
    synchronized (foos) {
      return ((Foo) foos.get(key));
    }
  }

  public void addFoo(Foo foo) {
    synchronized (foos) {
      foos.put(foo.getKey(), foo);
    }
  }

blocks only getFoo() and addFoo() calls from interfering with each other.

> Would you also agree that every call to
> session.setAttribute() and session.getAttribute() needs to
> be synchronized?

Absolutely not.  The container takes care of ensuring that this call is
thread safe on its internal implementation.

>
> Noah
>

Craig


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to