On Mon, 27 Jan 2003, David Graham wrote:
> Date: Mon, 27 Jan 2003 22:33:39 -0700 > From: David Graham <[EMAIL PROTECTED]> > Reply-To: Struts Developers List <[EMAIL PROTECTED]> > To: [EMAIL PROTECTED] > 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: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
