I guess you are right.... synchronization should be faster then doing a fs access here anyway.
Please commit :) Bye, Norman 2011/10/8 Felix Knecht <[email protected]>: > Wouldn't it make more sense to synchronize on queueF? There's BTW the same > problem for deleteFile (see first part of diff). > > Regards > Felix > > Index: > src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java > =================================================================== > --- > src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java > (revision 1180096) > +++ > src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java > (working copy) > @@ -104,8 +104,8 @@ > */ > public void deleteFile(ActiveMQBlobMessage message) throws IOException, > JMSException { > File f = getFile(message); > - if (f.exists()) { > - if (f.delete() == false) { > + synchronized (f) { > + if (f.exists() && !f.delete()) { > throw new IOException("Unable to delete file " + f); > } > } > @@ -143,20 +143,12 @@ > > File queueF = fs.getFile(queueUrl); > > - // check if we need to create the queue folder > - if (!queueF.exists()) { > - if (!queueF.mkdirs()) { > - // It could be that queueF.mkdirs() returned false because > - // queueF has been created > - // in the meantime (eg. by a different thread). Only throw > an > - // exception if this is > - // not the case. > - if (!queueF.exists()) { > - throw new IOException("Unable to create directory " + > queueF.getAbsolutePath()); > - } > + synchronized (queueF) { > + // check if we need to create the queue folder > + if (!queueF.exists() && !queueF.mkdirs()) { > + throw new IOException("Unable to create directory " + > queueF.getAbsolutePath()); > } > - > - } > + } > > return fs.getFile(queueUrl + "/" + filename); > > > On 10/07/2011 06:42 PM, [email protected] wrote: >> >> Author: norman >> Date: Fri Oct 7 16:42:36 2011 >> New Revision: 1180096 >> >> URL: http://svn.apache.org/viewvc?rev=1180096&view=rev >> Log: >> Make getFile(...) method of FileSystemBlobStrategy thread-safe. Thanks to >> Michael Herrmann for the patch. See JAMES-1327 >> >> Modified: >> >> james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java >> >> Modified: >> james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java >> URL: >> http://svn.apache.org/viewvc/james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java?rev=1180096&r1=1180095&r2=1180096&view=diff >> >> ============================================================================== >> --- >> james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java >> (original) >> +++ >> james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java >> Fri Oct 7 16:42:36 2011 >> @@ -144,11 +144,20 @@ public class FileSystemBlobStrategy impl >> File queueF = fs.getFile(queueUrl); >> >> // check if we need to create the queue folder >> - if (queueF.exists() == false) { >> + if (!queueF.exists()) { >> if (!queueF.mkdirs()) { >> - throw new IOException("Unable to create directory " + >> queueF.getAbsolutePath()); >> + // It could be that queueF.mkdirs() returned false >> because >> + // queueF has been created >> + // in the meantime (eg. by a different thread). Only >> throw an >> + // exception if this is >> + // not the case. >> + if (!queueF.exists()) { >> + throw new IOException("Unable to create directory " + >> queueF.getAbsolutePath()); >> + } >> } >> - } >> + >> + } >> + >> return fs.getFile(queueUrl + "/" + filename); >> >> } >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
