james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Knecht <fel...@apache.org>
Subject Re: svn commit: r1180096 - /james/server/trunk/queue-activemq/src/main/java/org/apache/james/queue/activemq/FileSystemBlobStrategy.java
Date Sat, 08 Oct 2011 16:50:29 GMT
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, norman@apache.org 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: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message