Fix a race condition in the HistoryService

The check for existing history files was performed outside of the lock, so multiple threads attempted to create them and failed. This lead to missing entries in the logs.

6 files changed, 13 insertions, 81 deletions

diff --git a/src/net/java/sip/communicator/impl/callhistory/CallHistoryServiceImpl.java b/src/net/java/sip/communicator/impl/callhistory/CallHistoryServiceImpl.java
index 117fb34..f0214f4 100644
--- a/src/net/java/sip/communicator/impl/callhistory/CallHistoryServiceImpl.java
+++ b/src/net/java/sip/communicator/impl/callhistory/CallHistoryServiceImpl.java
@@ -333,8 +333,6 @@ public class CallHistoryServiceImpl
      */
     private History getHistory(Contact localContact, Contact remoteContact)
             throws IOException {
-        History retVal = null;
-
         String localId = localContact == null ? "default" : localContact
                 .getAddress();
         String remoteId = remoteContact == null ? "default" : remoteContact
@@ -345,18 +343,7 @@ public class CallHistoryServiceImpl
                             localId,
                             remoteId });
 
-        if (this.historyService.isHistoryExisting(historyId))
-        {
-            retVal = this.historyService.getHistory(historyId);
-            retVal.setHistoryRecordsStructure(recordStructure);
-        }
-        else
-        {
-            retVal = this.historyService.createHistory(historyId,
-                    recordStructure);
-        }
-
-        return retVal;
+        return this.historyService.createHistory(historyId, recordStructure);
     }
 
     /**
diff --git a/src/net/java/sip/communicator/impl/filehistory/FileHistoryServiceImpl.java b/src/net/java/sip/communicator/impl/filehistory/FileHistoryServiceImpl.java
index 83fe513..0c35b6f 100644
--- a/src/net/java/sip/communicator/impl/filehistory/FileHistoryServiceImpl.java
+++ b/src/net/java/sip/communicator/impl/filehistory/FileHistoryServiceImpl.java
@@ -639,8 +639,6 @@ public class FileHistoryServiceImpl
     private History getHistory(Contact localContact, Contact remoteContact)
             throws IOException
     {
-        History retVal = null;
-
         String localId = localContact == null ? "default" : localContact
                 .getAddress();
         String remoteId = remoteContact == null ? "default" : remoteContact
@@ -657,16 +655,7 @@ public class FileHistoryServiceImpl
                             account,
                             remoteId });
 
-        if (this.historyService.isHistoryExisting(historyId))
-        {
-            retVal = this.historyService.getHistory(historyId);
-        } else
-        {
-            retVal = this.historyService.createHistory(historyId,
-                    recordStructure);
-        }
-
-        return retVal;
+        return this.historyService.createHistory(historyId, recordStructure);
     }
 
     /**
diff --git a/src/net/java/sip/communicator/impl/history/HistoryServiceImpl.java b/src/net/java/sip/communicator/impl/history/HistoryServiceImpl.java
index 09100a2..98a010f 100644
--- a/src/net/java/sip/communicator/impl/history/HistoryServiceImpl.java
+++ b/src/net/java/sip/communicator/impl/history/HistoryServiceImpl.java
@@ -172,9 +172,11 @@ public class HistoryServiceImpl
         {
             if (this.histories.containsKey(id))
             {
-                throw new IllegalArgumentException(
-                        "There is already a history with the specified ID.");
-            } else {
+                retVal = this.histories.get(id);
+                retVal.setHistoryRecordsStructure(recordStructure);
+            }
+            else
+            {
                 File dir = this.createHistoryDirectories(id);
                 HistoryImpl history = new HistoryImpl(id, dir, recordStructure,
                         this);
diff --git a/src/net/java/sip/communicator/impl/msghistory/MessageHistoryServiceImpl.java b/src/net/java/sip/communicator/impl/msghistory/MessageHistoryServiceImpl.java
index cd3872f..ee6da6b 100644
--- a/src/net/java/sip/communicator/impl/msghistory/MessageHistoryServiceImpl.java
+++ b/src/net/java/sip/communicator/impl/msghistory/MessageHistoryServiceImpl.java
@@ -371,16 +371,8 @@ public class MessageHistoryServiceImpl
         if(!this.historyService.isHistoryCreated(historyID))
             return false;
 
-        History history;
-        if (this.historyService.isHistoryExisting(historyID))
-        {
-            history = this.historyService.getHistory(historyID);
-        }
-        else
-        {
-            history = this.historyService.createHistory(historyID,
+        History history = this.historyService.createHistory(historyID,
                 recordStructure);
-        }
 
         return history.getReader().findLast(
             1, keywords, field, caseSensitive).hasNext();
@@ -451,16 +443,8 @@ public class MessageHistoryServiceImpl
                 if(descriptor == null)
                     continue;
 
-                History history;
-                if (this.historyService.isHistoryExisting(id))
-                {
-                    history = this.historyService.getHistory(id);
-                }
-                else
-                {
-                    history = this.historyService.createHistory(id,
+                History history = this.historyService.createHistory(id,
                         recordStructure);
-                }
 
                 HistoryReader reader = history.getReader();
 
@@ -753,8 +737,6 @@ public class MessageHistoryServiceImpl
      */
     private History getHistory(Contact localContact, Contact remoteContact)
             throws IOException {
-        History retVal = null;
-
         String localId = localContact == null ? "default" : localContact
                 .getAddress();
         String remoteId = remoteContact == null ? "default" : remoteContact
@@ -797,17 +779,7 @@ public class MessageHistoryServiceImpl
             }
         }
 
-        if (this.historyService.isHistoryExisting(historyId))
-        {
-            retVal = this.historyService.getHistory(historyId);
-        }
-        else
-        {
-            retVal = this.historyService.createHistory(historyId,
-                recordStructure);
-        }
-
-        return retVal;
+        return this.historyService.createHistory(historyId, recordStructure);
     }
 
     /**
@@ -874,8 +846,6 @@ public class MessageHistoryServiceImpl
                         String channel)
         throws IOException
     {
-        History retVal = null;
-
         String localId = localContact == null ? "default" : localContact
                 .getAddress();
 
@@ -885,15 +855,7 @@ public class MessageHistoryServiceImpl
                             account,
                             channel + "@" + server });
 
-        if (this.historyService.isHistoryExisting(historyId))
-        {
-            retVal = this.historyService.getHistory(historyId);
-        } else {
-            retVal = this.historyService.createHistory(historyId,
-                    recordStructure);
-        }
-
-        return retVal;
+        return this.historyService.createHistory(historyId, recordStructure);
     }
 
     /**
diff --git a/src/net/java/sip/communicator/impl/msghistory/MessageSourceService.java b/src/net/java/sip/communicator/impl/msghistory/MessageSourceService.java
index cab702e..bd18707 100644
--- a/src/net/java/sip/communicator/impl/msghistory/MessageSourceService.java
+++ b/src/net/java/sip/communicator/impl/msghistory/MessageSourceService.java
@@ -705,10 +705,7 @@ public class MessageSourceService
 
             if(history == null)
             {
-                if (historyService.isHistoryExisting(historyID))
-                    history = historyService.getHistory(historyID);
-                else
-                    history = historyService.createHistory(
+                history = historyService.createHistory(
                         historyID, recordStructure);
 
                 // lets check the version if not our version, re-create
diff --git a/test/net/java/sip/communicator/slick/history/TestHistoryService.java b/test/net/java/sip/communicator/slick/history/TestHistoryService.java
index 74ad5fb..c653e85 100644
--- a/test/net/java/sip/communicator/slick/history/TestHistoryService.java
+++ b/test/net/java/sip/communicator/slick/history/TestHistoryService.java
@@ -63,13 +63,8 @@ public class TestHistoryService extends TestCase {
         HistoryID testID = HistoryID.createFromRawID(new String[] { "test",
                 "alltests" });
 
-        if (!this.historyService.isHistoryExisting(testID))
-        {
-            this.history = this.historyService.createHistory(testID,
+        this.history = this.historyService.createHistory(testID,
                     recordStructure);
-        } else {
-            this.history = this.historyService.getHistory(testID);
-        }
     }
 
     @Override