You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by David Graham <dg...@hotmail.com> on 2003/01/28 06:33:39 UTC
Synchronized blocks?
Maybe I'm displaying gross ignorance here but AFAIK a synchronized block
grabs the object's monitor and blocks other threads from calling
*synchronized* methods on that object, right? Unsynchronized methods on
that object can be called at will.
I'm looking at LookupDispatchAction for PR #16019 and it has some strange
synchronized blocks. It synchronizes on HashMap objects which contain no
synchronized methods. Can anyone explain this to me?
I believe the Maps should be defined as Collections.synchronizedMap(new
HashMap()); This alleviates the need for one of the synchronized blocks but
the other is still needed because it Iterates over the key Set.
Thanks,
Dave
_________________________________________________________________
The new MSN 8: advanced junk mail protection and 2 months FREE*
http://join.msn.com/?page=features/junkmail
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: Synchronized blocks?
Posted by Mark Galbreath <ma...@qat.com>.
I've read his book - good one.
Mark
-----Original Message-----
From: news [mailto:news@main.gmane.org] On Behalf Of V. Cekvenich
Sent: Tuesday, January 28, 2003 8:21 AM
Best site on collections and threads:
http://gee.cs.oswego.edu/dl/classes/collections/
by Doug Lea!!!!
.V
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Synchronized blocks?
Posted by "V. Cekvenich" <vc...@basebeans.com>.
Best site on collections and threads:
http://gee.cs.oswego.edu/dl/classes/collections/
by Doug Lea!!!!
.V
David Graham wrote:
> Maybe I'm displaying gross ignorance here but AFAIK a synchronized block
> grabs the object's monitor and blocks other threads from calling
> *synchronized* methods on that object, right? Unsynchronized methods on
> that object can be called at will.
>
> I'm looking at LookupDispatchAction for PR #16019 and it has some
> strange synchronized blocks. It synchronizes on HashMap objects which
> contain no synchronized methods. Can anyone explain this to me?
>
> I believe the Maps should be defined as Collections.synchronizedMap(new
> HashMap()); This alleviates the need for one of the synchronized blocks
> but the other is still needed because it Iterates over the key Set.
>
> Thanks,
> Dave
>
>
>
>
>
> _________________________________________________________________
> The new MSN 8: advanced junk mail protection and 2 months FREE*
> http://join.msn.com/?page=features/junkmail
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Synchronized blocks?
Posted by "Craig R. McClanahan" <cr...@apache.org>.
On Mon, 27 Jan 2003, David Graham wrote:
> Maybe I'm displaying gross ignorance here but AFAIK a synchronized block
> grabs the object's monitor and blocks other threads from calling
> *synchronized* methods on that object, right? Unsynchronized methods on
> that object can be called at will.
To further illustrate this situation (for the record, in the archives).
The following two approaches to synchronizing access to a local variable
have *exactly* the same effect:
public class Foo {
private Map bar = new HashMap();
public synchronized void modifyBar() {
... do something that accesses bar ...
}
public differentMethod() {
... do something else ...
}
}
public class Foo {
private Map bar = new HashMap();
public void modifyBar() {
synchronized (this) {
... do something that accesses bar ...
}
}
public differentMethod() {
... do something else ...
}
}
Note that *neither* approach causes calls to differentMethod() to be
blocked, so if differentMethod() also accesses bar, then you're in
trouble.
In that sort of scenario, I prefer to explicitly lock the collection I'm
accessing instead, and do so for the absolute minimum amount of time.
Consider this class:
public class Foo {
private Map bar = new HashMap();
private Map baz = new HashMap();
public void modifyBar() {
... stuff that does NOT access bar or baz ...
synchronized (bar) {
... do something that accesses bar ...
}
... stuff that does NOT access bar or baz ...
}
public differentMethod() {
... stuff that does NOT access bar or baz ...
synchronized(bar) {
... do something that accesses bar ...
}
... stuff that does NOT access bar or baz ...
}
public anotherMethod() {
... stuff that does NOT access bar or baz...
synchronized(baz) {
... do something that accesses baz ...
}
... stuff that does NOT access bar or baz ...
}
}
Note that this latter example locks explicitly on the "bar" or "baz"
variable, instead of on "this" (i.e. the Foo instance). Because of that,
we experience several benefits to our performance in a multithreaded
environment:
* The "stuff that does NOT access bar or baz" chunks of code are
never blocked by a lock.
* Calls to differentMethod() never block calls to anotherMethod(),
and vice versa.
If you synchronized all three methods, you would indeed achieve thread
safety, but at the cost of calls to any one method blocking calls to the
other two (remember, they'd all effectively be synchronizing on "this").
If you used Collections.synchronizedMap() around bar and baz, you wouldn't
need synchronization blocks in the methods, but you'd cause multiple locks
to occur if there were more than one statement accessing the underlying
collection inside the blocks.
Note also that the caller of any of these methods does not have to be
concerned about thread safety -- the Foo class takes care of protecting
itself already. In general, that is a better design philosophy than
shoving the responsibility back o the user -- except for cases (such as
HashMap) where you explicitly *want* the caller to have the opportunity to
use your class with no locks at all, such as in a single threaded batch
application. Indeed, this is why HashMap is more performant than
Hashtable -- locks are only required when you're multithreading acces to
it. But the downside is that it is *your* responsibility, not the JDK"s,
to figure out when this is needed.
Craig
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: Synchronized blocks?
Posted by "Craig R. McClanahan" <cr...@apache.org>.
On Mon, 27 Jan 2003, David Graham wrote:
> Date: Mon, 27 Jan 2003 22:33:39 -0700
> From: David Graham <dg...@hotmail.com>
> Reply-To: Struts Developers List <st...@jakarta.apache.org>
> To: struts-dev@jakarta.apache.org
> Subject: Synchronized blocks?
>
> Maybe I'm displaying gross ignorance here but AFAIK a synchronized block
> grabs the object's monitor and blocks other threads from calling
> *synchronized* methods on that object, right? Unsynchronized methods on
> that object can be called at will.
>
No ... it only locks out other users that synchronize on the same object.
Thread safety is an "everyone must obey the same rules" sort of thing.
In the case at hand, the collections being locked are not publicly
visible, so there is no possibility for any other thread to access them
*except* by going through the same LookupDispatchAction instance. It
would be a different story if there as a public getLocaleMap() method, or
something like that.
> I'm looking at LookupDispatchAction for PR #16019 and it has some strange
> synchronized blocks. It synchronizes on HashMap objects which contain no
> synchronized methods. Can anyone explain this to me?
>
You cannot tell from the method signatures whether HashMap is threadsafe
or not simply because of whether the methods have a "synchronized"
modifier or not. It's entirely possible for a method to use internal
synchronized blocks in the way that LookupDispatchAction does -- although
that is ***not*** the case for HashMap, as the JDK JavaDocs will tell you.
> I believe the Maps should be defined as Collections.synchronizedMap(new
> HashMap()); This alleviates the need for one of the synchronized blocks but
> the other is still needed because it Iterates over the key Set.
>
That works, as long as you're willing to pay the synchronized price on
every single access to the collection from anywhere.
In the case of LookupDispatchAction, it looks like Erik was following the
"best practice" for multi-thread aware programs to synchronize for the
absolute minimum time period necessary. There are two synchronized blocks
here:
* The "synchronized (localeMap)" block only locks the
localeMap lock for long enough to create a new entry
if needed.
* The "synchronzized (lookupMap)" block only locks the
new localeMap entry (but not all of the rest of the
localeMap entries) for long enough to populate the new
Map.
A synchronizedMap lock around localeMap would needlessly slow down users
of *other* localeMap entries while a new one was being populated (which
could take a while, depending on which MessageResources implementation
class is used.
This class looks threadsafe, as is, to me.
> Thanks,
> Dave
Craig
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>