Agreed; the entire method has to be synchronized to avoid double checked logging issues
On 22/04/2008, Roman Kalukiewicz <[EMAIL PROTECTED]> wrote: > 2008/4/22, [EMAIL PROTECTED] <[EMAIL PROTECTED]>: > > > Author: hadrian > > Date: Mon Apr 21 21:26:05 2008 > > New Revision: 650379 > > > > URL: http://svn.apache.org/viewvc?rev=650379&view=rev > > Log: > > Fix thread safety issue. > > > > Modified: > > > activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java > > > > Modified: > activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java > > URL: > http://svn.apache.org/viewvc/activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java?rev=650379&r1=650378&r2=650379&view=diff > > > ============================================================================== > > --- > activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java > (original) > > +++ > activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java > Mon Apr 21 21:26:05 2008 > > @@ -58,7 +58,7 @@ > > private static final String DEFAULT_QUEUE_BROWSE_STRATEGY = > "org.apache.camel.component.jms.DefaultQueueBrowseStrategy"; > > private JmsConfiguration configuration; > > private ApplicationContext applicationContext; > > - private Requestor requestor; > > + private volatile Requestor requestor; > > private QueueBrowseStrategy queueBrowseStrategy; > > private boolean attemptedToCreateQueueBrowserStrategy; > > > > @@ -314,8 +314,10 @@ > > > > public synchronized Requestor getRequestor() throws Exception { > > if (requestor == null) { > > - requestor = new Requestor(getConfiguration(), > getExecutorService()); > > - requestor.start(); > > + synchronized (this) { > > + requestor = new Requestor(getConfiguration(), > getExecutorService()); > > + requestor.start(); > > + } > > } > > return requestor; > > } > > > The method is synchronized anyway so we don't need > 'synchronized(this)' I believe. > If it was not synchronized we anyway could start requestor twice (as > two threads check requestor == null, then both could create and start > new requestor. > > I believe the code was fine before and we don't need this change. > > > Roman > -- James ------- http://macstrac.blogspot.com/ Open Source Integration http://open.iona.com
