changeset 5420:d37dff2c0028 am-new-cache

Optimized the MemoryStringCache to prevent loading the same file multiple times if multiple users request the same file at the same time
author Alain Mazy <am@osimis.io>
date Mon, 13 Nov 2023 17:01:59 +0100
parents ac4e9fb87615
children e2c9f9d9700e
files NEWS OrthancFramework/Sources/Cache/MemoryStringCache.cpp OrthancFramework/Sources/Cache/MemoryStringCache.h OrthancFramework/Sources/FileStorage/StorageAccessor.cpp OrthancFramework/Sources/FileStorage/StorageAccessor.h OrthancFramework/Sources/FileStorage/StorageCache.cpp OrthancFramework/Sources/FileStorage/StorageCache.h OrthancFramework/UnitTestsSources/MemoryCacheTests.cpp OrthancServer/Sources/ServerContext.cpp
diffstat 9 files changed, 711 insertions(+), 175 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Thu Nov 09 08:51:01 2023 +0100
+++ b/NEWS	Mon Nov 13 17:01:59 2023 +0100
@@ -8,6 +8,8 @@
   - Allow multiple plugins to use the plugin SDK at the same time.  In previous versions,
     functions like instance transcoding or instance reading where mutually exclusive.
     This can bring some significant improvements particularly in viewers.
+  - Optimized the StorageCache to prevent loading the same file multiple times if
+    multiple users request the same file at the same time.
 * Housekeeper plugin:
   - Update to rebuild the cache of the DicomWeb plugin when updating to DicomWeb 1.15.
   - New trigger configuration: "DicomWebCacheChange"
--- a/OrthancFramework/Sources/Cache/MemoryStringCache.cpp	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/Cache/MemoryStringCache.cpp	Mon Nov 13 17:01:59 2023 +0100
@@ -53,47 +53,221 @@
     }      
   };
 
+
+  MemoryStringCache::Accessor::Accessor(MemoryStringCache& cache)
+  : cache_(cache),
+    shouldAdd_(false)
+  {
+  }
+
+
+  MemoryStringCache::Accessor::~Accessor()
+  {
+    // if this accessor was the one in charge of loading and adding the data into the cache
+    // and it failed to add, remove the key from the list to make sure others accessor
+    // stop waiting for it.
+    if (shouldAdd_)
+    {
+      cache_.RemoveFromItemsBeingLoaded(keyToAdd_);
+    }
+  }
+
+
+  bool MemoryStringCache::Accessor::Fetch(std::string& value, const std::string& key)
+  {
+    // if multiple accessors are fetching at the same time:
+    // the first one will return false and will be in charge of adding to the cache.
+    // others will wait.
+    // if the first one fails to add, or, if the content was too large to fit in the cache,
+    // the next one will be in charge of adding ...
+    if (!cache_.Fetch(value, key))
+    {
+      shouldAdd_ = true;
+      keyToAdd_ = key;
+      return false;
+    }
+
+    shouldAdd_ = false;
+    keyToAdd_.clear();
+
+    return true;
+  }
+
+
+  void MemoryStringCache::Accessor::Add(const std::string& key, const std::string& value)
+  {
+    cache_.Add(key, value);
+    shouldAdd_ = false;
+  }
+
+
+  void MemoryStringCache::Accessor::Add(const std::string& key, const char* buffer, size_t size)
+  {
+    cache_.Add(key, buffer, size);
+    shouldAdd_ = false;
+  }
+
+
+  MemoryStringCache::MemoryStringCache() :
+    currentSize_(0),
+    maxSize_(100 * 1024 * 1024)  // 100 MB
+  {
+  }
+
+
+  MemoryStringCache::~MemoryStringCache()
+  {
+    Recycle(0);
+    assert(content_.IsEmpty());
+  }
+
+
   size_t MemoryStringCache::GetMaximumSize()
   {
-    return cache_.GetMaximumSize();
+    return maxSize_;
   }
 
+
   void MemoryStringCache::SetMaximumSize(size_t size)
   {
-    cache_.SetMaximumSize(size);
+    if (size == 0)
+    {
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+      
+    // // Make sure no accessor is currently open (as its data may be
+    // // removed if recycling is needed)
+    // WriterLock contentLock(contentMutex_);
+
+    // Lock the global structure of the cache
+    boost::mutex::scoped_lock cacheLock(cacheMutex_);
+
+    Recycle(size);
+    maxSize_ = size;
   }
 
+
   void MemoryStringCache::Add(const std::string& key,
-                              const std::string& value)
+                               const std::string& value)
   {
-    cache_.Acquire(key, new StringValue(value));
+    std::unique_ptr<StringValue> item(new StringValue(value));
+    size_t size = value.size();
+
+    boost::mutex::scoped_lock cacheLock(cacheMutex_);
+
+    if (size > maxSize_)
+    {
+      // This object is too large to be stored in the cache, discard it
+    }
+    else if (content_.Contains(key))
+    {
+      // Value already stored, don't overwrite the old value but put it on top of the cache
+      content_.MakeMostRecent(key);
+    }
+    else
+    {
+      Recycle(maxSize_ - size);   // Post-condition: currentSize_ <= maxSize_ - size
+      assert(currentSize_ + size <= maxSize_);
+
+      content_.Add(key, item.release());
+      currentSize_ += size;
+    }
+
+    RemoveFromItemsBeingLoadedInternal(key);
   }
 
+
   void MemoryStringCache::Add(const std::string& key,
                               const void* buffer,
                               size_t size)
   {
-    cache_.Acquire(key, new StringValue(reinterpret_cast<const char*>(buffer), size));
+    Add(key, std::string(reinterpret_cast<const char*>(buffer), size));
   }
 
+
   void MemoryStringCache::Invalidate(const std::string &key)
   {
-    cache_.Invalidate(key);
+    boost::mutex::scoped_lock cacheLock(cacheMutex_);
+
+    StringValue* item = NULL;
+    if (content_.Contains(key, item))
+    {
+      assert(item != NULL);
+      const size_t size = item->GetMemoryUsage();
+      delete item;
+
+      content_.Invalidate(key);
+          
+      assert(currentSize_ >= size);
+      currentSize_ -= size;
+    }
+
+    RemoveFromItemsBeingLoadedInternal(key);
   }
-  
+
+
   bool MemoryStringCache::Fetch(std::string& value,
                                 const std::string& key)
   {
-    MemoryObjectCache::Accessor reader(cache_, key, false /* multiple readers are allowed */);
+    boost::mutex::scoped_lock cacheLock(cacheMutex_);
+
+    StringValue* item;
 
-    if (reader.IsValid())
+    // if another client is currently loading the item, wait for it.
+    while (itemsBeingLoaded_.find(key) != itemsBeingLoaded_.end() && !content_.Contains(key, item))
     {
-      value = dynamic_cast<StringValue&>(reader.GetValue()).GetContent();
+      cacheCond_.wait(cacheLock);
+    }
+
+    if (content_.Contains(key, item))
+    {
+      value = dynamic_cast<StringValue&>(*item).GetContent();
+      content_.MakeMostRecent(key);
+
       return true;
     }
     else
     {
+      // note that this accessor will be in charge of loading and adding.
+      itemsBeingLoaded_.insert(key);
       return false;
     }
   }
+
+
+  void MemoryStringCache::RemoveFromItemsBeingLoaded(const std::string& key)
+  {
+    boost::mutex::scoped_lock cacheLock(cacheMutex_);
+    RemoveFromItemsBeingLoadedInternal(key);
+  }
+
+
+  void MemoryStringCache::RemoveFromItemsBeingLoadedInternal(const std::string& key)
+  {
+    // notify all waiting users, some of them potentially waiting for this item
+    itemsBeingLoaded_.erase(key);
+    cacheCond_.notify_all();
+  }
+
+  void MemoryStringCache::Recycle(size_t targetSize)
+  {
+    // WARNING: "cacheMutex_" must be locked
+    while (currentSize_ > targetSize)
+    {
+      assert(!content_.IsEmpty());
+        
+      StringValue* item = NULL;
+      content_.RemoveOldest(item);
+
+      assert(item != NULL);
+      const size_t size = item->GetMemoryUsage();
+      delete item;
+
+      assert(currentSize_ >= size);
+      currentSize_ -= size;
+    }
+
+    // Post-condition: "currentSize_ <= targetSize"
+
+  }
 }
--- a/OrthancFramework/Sources/Cache/MemoryStringCache.h	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/Cache/MemoryStringCache.h	Mon Nov 13 17:01:59 2023 +0100
@@ -23,28 +23,71 @@
 
 #pragma once
 
-#include "MemoryObjectCache.h"
+#include "../OrthancFramework.h"
+#include "ICacheable.h"
+#include "LeastRecentlyUsedIndex.h"
+
+#include <boost/thread/condition_variable.hpp>
+#include <boost/thread/mutex.hpp>
+
 
 namespace Orthanc
 {
   /**
-   * Facade object around "MemoryObjectCache" that caches a dictionary
+   * Class that caches a dictionary
    * of strings, using the "fetch/add" paradigm of memcached.
    * 
+   * Starting from 1.12.2, if multiple clients are trying to access
+   * an inexistent item at the same time, only one of them will load it
+   * and the others will wait until the first one has loaded the data.
+   * 
+   * The MemoryStringCache is only accessible through an Accessor.
+   * 
    * Note: this class is thread safe
    **/
   class ORTHANC_PUBLIC MemoryStringCache : public boost::noncopyable
   {
+  public:
+    class Accessor : public boost::noncopyable
+    {
+      MemoryStringCache& cache_;
+      bool                shouldAdd_;  // when this accessor is the one who should load and add the data
+      std::string         keyToAdd_;
+    public:
+      Accessor(MemoryStringCache& cache);
+      ~Accessor();
+
+      bool Fetch(std::string& value, const std::string& key);
+      void Add(const std::string& key, const std::string& value);
+      void Add(const std::string& key,const char* buffer, size_t size);
+    };
+
   private:
     class StringValue;
 
-    MemoryObjectCache  cache_;
+    boost::mutex              cacheMutex_;  // note: we can not use recursive_mutex with condition_variable
+    boost::condition_variable cacheCond_;
+    std::set<std::string>     itemsBeingLoaded_;
+
+    size_t currentSize_;
+    size_t maxSize_;
+    LeastRecentlyUsedIndex<std::string, StringValue*>  content_;
+
+    void Recycle(size_t targetSize);
 
   public:
+    MemoryStringCache();
+
+    ~MemoryStringCache();
+
     size_t GetMaximumSize();
     
     void SetMaximumSize(size_t size);
 
+    void Invalidate(const std::string& key);
+
+
+  private:
     void Add(const std::string& key,
              const std::string& value);
 
@@ -52,9 +95,12 @@
              const void* buffer,
              size_t size);
 
-    void Invalidate(const std::string& key);
-
     bool Fetch(std::string& value,
                const std::string& key);
+
+    void RemoveFromItemsBeingLoaded(const std::string& key);
+    void RemoveFromItemsBeingLoadedInternal(const std::string& key);
+
+    void AddToItemsBeingLoadedInternal(const std::string& key);
   };
 }
--- a/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp	Mon Nov 13 17:01:59 2023 +0100
@@ -130,7 +130,8 @@
         
         if (cache_ != NULL)
         {
-          cache_->Add(uuid, type, data, size);
+          StorageCache::Accessor cacheAccessor(*cache_);
+          cacheAccessor.Add(uuid, type, data, size);
         }
 
         return FileInfo(uuid, type, size, md5);
@@ -170,7 +171,8 @@
 
         if (cache_ != NULL)
         {
-          cache_->Add(uuid, type, data, size);  // always add uncompressed data to cache
+          StorageCache::Accessor cacheAccessor(*cache_);
+          cacheAccessor.Add(uuid, type, data, size);    // always add uncompressed data to cache
         }
 
         return FileInfo(uuid, type, size, md5,
@@ -195,61 +197,72 @@
   void StorageAccessor::Read(std::string& content,
                              const FileInfo& info)
   {
-    if (cache_ == NULL ||
-        !cache_->Fetch(content, info.GetUuid(), info.GetContentType()))
+    if (cache_ == NULL)
+    {
+      ReadWholeInternal(content, info);
+    }
+    else
     {
-      switch (info.GetCompressionType())
+      StorageCache::Accessor cacheAccessor(*cache_);
+
+      if (!cacheAccessor.Fetch(content, info.GetUuid(), info.GetContentType()))
       {
-        case CompressionType_None:
-        {
-          std::unique_ptr<IMemoryBuffer> buffer;
+        ReadWholeInternal(content, info);
 
-          {
-            MetricsTimer timer(*this, METRICS_READ_DURATION);
-            buffer.reset(area_.Read(info.GetUuid(), info.GetContentType()));
-          }
+        // always store the uncompressed data in cache
+        cacheAccessor.Add(info.GetUuid(), info.GetContentType(), content);
+      }
+    }
+  }
 
-          if (metrics_ != NULL)
-          {
-            metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
-          }
+  void StorageAccessor::ReadWholeInternal(std::string& content,
+                                          const FileInfo& info)
+  {
+    switch (info.GetCompressionType())
+    {
+      case CompressionType_None:
+      {
+        std::unique_ptr<IMemoryBuffer> buffer;
 
-          buffer->MoveToString(content);
-
-          break;
+        {
+          MetricsTimer timer(*this, METRICS_READ_DURATION);
+          buffer.reset(area_.Read(info.GetUuid(), info.GetContentType()));
         }
 
-        case CompressionType_ZlibWithSize:
+        if (metrics_ != NULL)
         {
-          ZlibCompressor zlib;
-
-          std::unique_ptr<IMemoryBuffer> compressed;
-          
-          {
-            MetricsTimer timer(*this, METRICS_READ_DURATION);
-            compressed.reset(area_.Read(info.GetUuid(), info.GetContentType()));
-          }
-          
-          if (metrics_ != NULL)
-          {
-            metrics_->IncrementIntegerValue(METRICS_READ_BYTES, compressed->GetSize());
-          }
-
-          zlib.Uncompress(content, compressed->GetData(), compressed->GetSize());
-
-          break;
+          metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
         }
 
-        default:
-        {
-          throw OrthancException(ErrorCode_NotImplemented);
-        }
+        buffer->MoveToString(content);
+
+        break;
       }
 
-      // always store the uncompressed data in cache
-      if (cache_ != NULL)
+      case CompressionType_ZlibWithSize:
       {
-        cache_->Add(info.GetUuid(), info.GetContentType(), content);
+        ZlibCompressor zlib;
+
+        std::unique_ptr<IMemoryBuffer> compressed;
+        
+        {
+          MetricsTimer timer(*this, METRICS_READ_DURATION);
+          compressed.reset(area_.Read(info.GetUuid(), info.GetContentType()));
+        }
+        
+        if (metrics_ != NULL)
+        {
+          metrics_->IncrementIntegerValue(METRICS_READ_BYTES, compressed->GetSize());
+        }
+
+        zlib.Uncompress(content, compressed->GetData(), compressed->GetSize());
+
+        break;
+      }
+
+      default:
+      {
+        throw OrthancException(ErrorCode_NotImplemented);
       }
     }
 
@@ -260,22 +273,39 @@
   void StorageAccessor::ReadRaw(std::string& content,
                                 const FileInfo& info)
   {
-    if (cache_ == NULL || !cache_->Fetch(content, info.GetUuid(), info.GetContentType()))
+    if (cache_ == NULL || info.GetCompressionType() != CompressionType_None)
     {
-      std::unique_ptr<IMemoryBuffer> buffer;
+      ReadRawInternal(content, info);
+    }
+    else
+    {// use the cache only if the data is uncompressed.
+      StorageCache::Accessor cacheAccessor(*cache_);
 
+      if (!cacheAccessor.Fetch(content, info.GetUuid(), info.GetContentType()))
       {
-        MetricsTimer timer(*this, METRICS_READ_DURATION);
-        buffer.reset(area_.Read(info.GetUuid(), info.GetContentType()));
+        ReadRawInternal(content, info);
+
+        cacheAccessor.Add(info.GetUuid(), info.GetContentType(), content);
       }
+    }
+  }
 
-      if (metrics_ != NULL)
-      {
-        metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
-      }
+  void StorageAccessor::ReadRawInternal(std::string& content,
+                                        const FileInfo& info)
+  {
+    std::unique_ptr<IMemoryBuffer> buffer;
 
-      buffer->MoveToString(content);
+    {
+      MetricsTimer timer(*this, METRICS_READ_DURATION);
+      buffer.reset(area_.Read(info.GetUuid(), info.GetContentType()));
     }
+
+    if (metrics_ != NULL)
+    {
+      metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
+    }
+
+    buffer->MoveToString(content);
   }
 
 
@@ -300,35 +330,70 @@
   }
 
 
+  void ReadStartRangeFromAreaInternal(std::string& target,
+                                      IStorageArea& area,
+                                      const std::string& fileUuid,
+                                      FileContentType contentType,
+                                      uint64_t end /* exclusive */)
+  {
+
+  }
+
   void StorageAccessor::ReadStartRange(std::string& target,
-                                       const std::string& fileUuid,
-                                       FileContentType contentType,
+                                       const FileInfo& info,
                                        uint64_t end /* exclusive */)
   {
-    if (cache_ == NULL || !cache_->FetchStartRange(target, fileUuid, contentType, end))
+    if (cache_ == NULL)
     {
-      std::unique_ptr<IMemoryBuffer> buffer;
-
+      ReadStartRangeInternal(target, info, end);
+    }
+    else
+    {
+      StorageCache::Accessor accessorStartRange(*cache_);
+      if (!accessorStartRange.FetchStartRange(target, info.GetUuid(), info.GetContentType(), end))
       {
-        MetricsTimer timer(*this, METRICS_READ_DURATION);
-        buffer.reset(area_.ReadRange(fileUuid, contentType, 0, end));
-        assert(buffer->GetSize() == end);
+        ReadStartRangeInternal(target, info, end);
+        accessorStartRange.AddStartRange(info.GetUuid(), info.GetContentType(), target);
       }
+      else
+      {
+        StorageCache::Accessor accessorWhole(*cache_);
+        if (!accessorWhole.Fetch(target, info.GetUuid(), info.GetContentType()))
+        {
+          ReadWholeInternal(target, info);
+          accessorWhole.Add(info.GetUuid(), info.GetContentType(), target);
+        }
 
-      if (metrics_ != NULL)
-      {
-        metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
-      }
+        if (target.size() < end)
+        {
+          throw OrthancException(ErrorCode_CorruptedFile);
+        }
 
-      buffer->MoveToString(target);
-
-      if (cache_ != NULL)
-      {
-        cache_->AddStartRange(fileUuid, contentType, target);
+        target.resize(end);
       }
     }
   }
 
+  void StorageAccessor::ReadStartRangeInternal(std::string& target,
+                                                const FileInfo& info,
+                                                uint64_t end /* exclusive */)
+  {
+    std::unique_ptr<IMemoryBuffer> buffer;
+
+    {
+      MetricsTimer timer(*this, METRICS_READ_DURATION);
+      buffer.reset(area_.ReadRange(info.GetUuid(), info.GetContentType(), 0, end));
+      assert(buffer->GetSize() == end);
+    }
+
+    if (metrics_ != NULL)
+    {
+      metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize());
+    }
+
+    buffer->MoveToString(target);
+  }
+
 
 #if ORTHANC_ENABLE_CIVETWEB == 1 || ORTHANC_ENABLE_MONGOOSE == 1
   void StorageAccessor::SetupSender(BufferHttpSender& sender,
--- a/OrthancFramework/Sources/FileStorage/StorageAccessor.h	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageAccessor.h	Mon Nov 13 17:01:59 2023 +0100
@@ -108,8 +108,7 @@
                  const FileInfo& info);
 
     void ReadStartRange(std::string& target,
-                        const std::string& fileUuid,
-                        FileContentType fullFileContentType,
+                        const FileInfo& info,
                         uint64_t end /* exclusive */);
 
     void Remove(const std::string& fileUuid,
@@ -134,5 +133,16 @@
                     const FileInfo& info,
                     const std::string& mime);
 #endif
+  private:
+    void ReadStartRangeInternal(std::string& target,
+                                const FileInfo& info,
+                                uint64_t end /* exclusive */);
+
+    void ReadWholeInternal(std::string& content,
+                           const FileInfo& info);
+
+    void ReadRawInternal(std::string& content,
+                         const FileInfo& info);
+
   };
 }
--- a/OrthancFramework/Sources/FileStorage/StorageCache.cpp	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageCache.cpp	Mon Nov 13 17:01:59 2023 +0100
@@ -44,41 +44,13 @@
   {
     return uuid + ":" + boost::lexical_cast<std::string>(contentType) + ":0";
   }
-  
+
   void StorageCache::SetMaximumSize(size_t size)
   {
     cache_.SetMaximumSize(size);
   }
   
 
-  void StorageCache::Add(const std::string& uuid, 
-                         FileContentType contentType,
-                         const std::string& value)
-  {
-    const std::string key = GetCacheKeyFullFile(uuid, contentType);
-    cache_.Add(key, value);
-  }
-  
-
-  void StorageCache::Add(const std::string& uuid, 
-                         FileContentType contentType,
-                         const void* buffer,
-                         size_t size)
-  {
-    const std::string key = GetCacheKeyFullFile(uuid, contentType);
-    cache_.Add(key, buffer, size);
-  }
-
-
-  void StorageCache::AddStartRange(const std::string& uuid, 
-                                   FileContentType contentType,
-                                   const std::string& value)
-  {
-    const std::string key = GetCacheKeyStartRange(uuid, contentType);
-    cache_.Add(key, value);
-  }
-
-
   void StorageCache::Invalidate(const std::string& uuid,
                                 FileContentType contentType)
   {
@@ -89,14 +61,45 @@
     const std::string keyPartialFile = GetCacheKeyStartRange(uuid, contentType);
     cache_.Invalidate(keyPartialFile);
   }
-  
+
+
+  StorageCache::Accessor::Accessor(StorageCache& cache)
+  : MemoryStringCache::Accessor(cache.cache_)
+  {
+  }
+
+  void StorageCache::Accessor::Add(const std::string& uuid, 
+                                   FileContentType contentType,
+                                   const std::string& value)
+  {
 
-  bool StorageCache::Fetch(std::string& value, 
-                           const std::string& uuid,
-                           FileContentType contentType)
+    std::string key = GetCacheKeyFullFile(uuid, contentType);
+    MemoryStringCache::Accessor::Add(key, value);
+  }
+
+  void StorageCache::Accessor::AddStartRange(const std::string& uuid, 
+                                             FileContentType contentType,
+                                             const std::string& value)
+  {
+    const std::string key = GetCacheKeyStartRange(uuid, contentType);
+    MemoryStringCache::Accessor::Add(key, value);
+  }
+
+  void StorageCache::Accessor::Add(const std::string& uuid, 
+                                   FileContentType contentType,
+                                   const void* buffer,
+                                   size_t size)
   {
     const std::string key = GetCacheKeyFullFile(uuid, contentType);
-    if (cache_.Fetch(value, key))
+    MemoryStringCache::Accessor::Add(key, reinterpret_cast<const char*>(buffer), size);
+  }                                   
+
+  bool StorageCache::Accessor::Fetch(std::string& value, 
+                                     const std::string& uuid,
+                                     FileContentType contentType)
+  {
+    const std::string key = GetCacheKeyFullFile(uuid, contentType);
+    if (MemoryStringCache::Accessor::Fetch(value, key))
     {
       LOG(INFO) << "Read attachment \"" << uuid << "\" with content type "
                 << boost::lexical_cast<std::string>(contentType) << " from cache";
@@ -108,14 +111,13 @@
     }
   }
 
-  bool StorageCache::FetchStartRange(std::string& value, 
-                                     const std::string& uuid,
-                                     FileContentType contentType,
-                                     uint64_t end)
+  bool StorageCache::Accessor::FetchStartRange(std::string& value, 
+                                               const std::string& uuid,
+                                               FileContentType contentType,
+                                               uint64_t end /* exclusive */)
   {
-    // first try to get the start of file only from cache
     const std::string keyPartialFile = GetCacheKeyStartRange(uuid, contentType);
-    if (cache_.Fetch(value, keyPartialFile) && value.size() >= end)
+    if (MemoryStringCache::Accessor::Fetch(value, keyPartialFile) && value.size() >= end)
     {
       if (value.size() > end)  // the start range that has been cached is larger than the requested value
       {
@@ -126,23 +128,7 @@
                 << boost::lexical_cast<std::string>(contentType) << " from cache";
       return true;
     }
-    else
-    {
-      // try to get the full file from cache
-      if (Fetch(value, uuid, contentType))
-      {
-        if (value.size() < end)
-        {
-          throw OrthancException(ErrorCode_CorruptedFile);
-        }
 
-        value.resize(end);
-        return true;
-      }
-      else
-      {
-        return false;
-      }
-    }
+    return false;
   }
 }
--- a/OrthancFramework/Sources/FileStorage/StorageCache.h	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/Sources/FileStorage/StorageCache.h	Mon Nov 13 17:01:59 2023 +0100
@@ -37,12 +37,51 @@
    **/
    class ORTHANC_PUBLIC StorageCache : public boost::noncopyable
     {
+    public:
+
+      // The StorageCache is only accessible through this accessor.
+      // It will make sure that only one user will fill load data and fill
+      // the cache if multiple users try to access the same item at the same time.
+      // This scenario happens a lot when multiple workers from a viewer access 
+      // the same file.
+      class Accessor : public MemoryStringCache::Accessor
+      {
+      public:
+        Accessor(StorageCache& cache);
+
+        void Add(const std::string& uuid, 
+                FileContentType contentType,
+                const std::string& value);
+
+        void AddStartRange(const std::string& uuid, 
+                          FileContentType contentType,
+                          const std::string& value);
+
+        void Add(const std::string& uuid, 
+                FileContentType contentType,
+                const void* buffer,
+                size_t size);
+
+        bool Fetch(std::string& value, 
+                  const std::string& uuid,
+                  FileContentType contentType);
+
+        bool FetchStartRange(std::string& value, 
+                            const std::string& uuid,
+                            FileContentType contentType,
+                            uint64_t end /* exclusive */);
+      };
+
     private:
       MemoryStringCache   cache_;
       
     public:
       void SetMaximumSize(size_t size);
 
+      void Invalidate(const std::string& uuid,
+                      FileContentType contentType);
+
+    private:
       void Add(const std::string& uuid, 
                FileContentType contentType,
                const std::string& value);
@@ -56,9 +95,6 @@
                const void* buffer,
                size_t size);
 
-      void Invalidate(const std::string& uuid,
-                      FileContentType contentType);
-
       bool Fetch(std::string& value, 
                  const std::string& uuid,
                  FileContentType contentType);
--- a/OrthancFramework/UnitTestsSources/MemoryCacheTests.cpp	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancFramework/UnitTestsSources/MemoryCacheTests.cpp	Mon Nov 13 17:01:59 2023 +0100
@@ -33,6 +33,7 @@
 #include "../Sources/Cache/SharedArchive.h"
 #include "../Sources/IDynamicObject.h"
 #include "../Sources/Logging.h"
+#include "../Sources/SystemToolbox.h"
 
 #include <memory>
 #include <algorithm>
@@ -319,44 +320,260 @@
   Orthanc::MemoryStringCache c;
   ASSERT_THROW(c.SetMaximumSize(0), Orthanc::OrthancException);
   
-  c.SetMaximumSize(2);
+  c.SetMaximumSize(3);
 
   std::string v;
-  ASSERT_FALSE(c.Fetch(v, "hello"));
+  {
+    Orthanc::MemoryStringCache::Accessor a(c);
+    ASSERT_FALSE(a.Fetch(v, "key1"));
+  }
 
-  c.Add("hello", "a");
-  ASSERT_TRUE(c.Fetch(v, "hello"));   ASSERT_EQ("a", v);
-  ASSERT_FALSE(c.Fetch(v, "hello2"));
-  ASSERT_FALSE(c.Fetch(v, "hello3"));
+  {
+    Orthanc::MemoryStringCache::Accessor a(c);
+    ASSERT_FALSE(a.Fetch(v, "key1"));
+    a.Add("key1", "a");
+    ASSERT_TRUE(a.Fetch(v, "key1"));
+    ASSERT_EQ("a", v);
 
-  c.Add("hello2", "b");
-  ASSERT_TRUE(c.Fetch(v, "hello"));   ASSERT_EQ("a", v);
-  ASSERT_TRUE(c.Fetch(v, "hello2"));  ASSERT_EQ("b", v);
-  ASSERT_FALSE(c.Fetch(v, "hello3"));
+    ASSERT_FALSE(a.Fetch(v, "key2"));
+    ASSERT_FALSE(a.Fetch(v, "key3"));
+
+    a.Add("key2", "b");
+    ASSERT_TRUE(a.Fetch(v, "key1"));
+    ASSERT_EQ("a", v);
+    ASSERT_TRUE(a.Fetch(v, "key2"));
+    ASSERT_EQ("b", v);
 
-  c.Add("hello3", "too large value");
-  ASSERT_TRUE(c.Fetch(v, "hello"));   ASSERT_EQ("a", v);
-  ASSERT_TRUE(c.Fetch(v, "hello2"));  ASSERT_EQ("b", v);
-  ASSERT_FALSE(c.Fetch(v, "hello3"));
-  
-  c.Add("hello3", "c");
-  ASSERT_FALSE(c.Fetch(v, "hello"));  // Recycled
-  ASSERT_TRUE(c.Fetch(v, "hello2"));  ASSERT_EQ("b", v);
-  ASSERT_TRUE(c.Fetch(v, "hello3"));  ASSERT_EQ("c", v);
+    a.Add("key3", "too-large-value");
+    ASSERT_TRUE(a.Fetch(v, "key1"));
+    ASSERT_EQ("a", v);
+    ASSERT_TRUE(a.Fetch(v, "key2"));
+    ASSERT_EQ("b", v);
+    ASSERT_FALSE(a.Fetch(v, "key3"));
+
+    a.Add("key3", "c");
+    ASSERT_TRUE(a.Fetch(v, "key2"));
+    ASSERT_EQ("b", v);
+    ASSERT_TRUE(a.Fetch(v, "key1"));
+    ASSERT_EQ("a", v);
+    ASSERT_TRUE(a.Fetch(v, "key3"));
+    ASSERT_EQ("c", v);
+
+    // adding a fourth value should remove the oldest accessed value (key2)
+    a.Add("key4", "d");
+    ASSERT_FALSE(a.Fetch(v, "key2"));
+    ASSERT_TRUE(a.Fetch(v, "key1"));
+    ASSERT_EQ("a", v);
+    ASSERT_TRUE(a.Fetch(v, "key3"));
+    ASSERT_EQ("c", v);
+    ASSERT_TRUE(a.Fetch(v, "key4"));
+    ASSERT_EQ("d", v);
+
+  }
 }
 
-
 TEST(MemoryStringCache, Invalidate)
 {
   Orthanc::MemoryStringCache c;
-  c.Add("hello", "a");
-  c.Add("hello2", "b");
+  Orthanc::MemoryStringCache::Accessor a(c);
+
+  a.Add("hello", "a");
+  a.Add("hello2", "b");
 
   std::string v;
-  ASSERT_TRUE(c.Fetch(v, "hello"));   ASSERT_EQ("a", v);
-  ASSERT_TRUE(c.Fetch(v, "hello2"));  ASSERT_EQ("b", v);
+  ASSERT_TRUE(a.Fetch(v, "hello"));   
+  ASSERT_EQ("a", v);
+  ASSERT_TRUE(a.Fetch(v, "hello2"));  
+  ASSERT_EQ("b", v);
 
   c.Invalidate("hello");
-  ASSERT_FALSE(c.Fetch(v, "hello"));
-  ASSERT_TRUE(c.Fetch(v, "hello2"));  ASSERT_EQ("b", v);
+  ASSERT_FALSE(a.Fetch(v, "hello"));
+  ASSERT_TRUE(a.Fetch(v, "hello2"));  
+  ASSERT_EQ("b", v);
+}
+
+
+static int ThreadingScenarioHappyStep = 0;
+static Orthanc::MemoryStringCache ThreadingScenarioHappyCache;
+
+void ThreadingScenarioHappyThread1()
+{
+  // the first thread to call Fetch (will be in charge of adding)
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioHappyCache);
+  std::string v;
+
+  LOG(INFO) << "Thread1 will fetch";
+  if (!a.Fetch(v, "key1"))
+  {
+    LOG(INFO) << "Thread1 has fetch";
+    ThreadingScenarioHappyStep = 1;
+    
+    // wait for the other thread to fetch too
+    while (ThreadingScenarioHappyStep < 2)
+    {
+      Orthanc::SystemToolbox::USleep(10000);
+    }
+    LOG(INFO) << "Thread1 will add after a short sleep";
+    Orthanc::SystemToolbox::USleep(100000);
+    LOG(INFO) << "Thread1 will add";
+
+    a.Add("key1", "value1");
+
+    LOG(INFO) << "Thread1 has added";
+  }
+}
+
+void ThreadingScenarioHappyThread2()
+{
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioHappyCache);
+  std::string v;
+
+  // nobody has added key2 -> go
+  if (!a.Fetch(v, "key2"))
+  {
+    a.Add("key2", "value2");
+  }
+
+  // wait until thread 1 has completed its "Fetch" but not added yet
+  while (ThreadingScenarioHappyStep < 1)
+  {
+    Orthanc::SystemToolbox::USleep(10000);
+  }
+
+  ThreadingScenarioHappyStep = 2;
+  LOG(INFO) << "Thread2 will fetch";
+  // this should wait until thread 1 has added
+  if (!a.Fetch(v, "key1"))
+  {
+    ASSERT_FALSE(true); // this thread should not add since thread1 should have done it
+  }
+  LOG(INFO) << "Thread2 has fetched the value";
+  ASSERT_EQ("value1", v);
+}
+
+
+TEST(MemoryStringCache, ThreadingScenarioHappy)
+{
+  boost::thread thread1 = boost::thread(ThreadingScenarioHappyThread1);
+  boost::thread thread2 = boost::thread(ThreadingScenarioHappyThread2);
+
+  thread1.join();
+  thread2.join();
 }
+
+
+static int ThreadingScenarioFailureStep = 0;
+static Orthanc::MemoryStringCache ThreadingScenarioFailureCache;
+
+void ThreadingScenarioFailureThread1()
+{
+  // the first thread to call Fetch (will be in charge of adding)
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioFailureCache);
+  std::string v;
+
+  LOG(INFO) << "Thread1 will fetch";
+  if (!a.Fetch(v, "key1"))
+  {
+    LOG(INFO) << "Thread1 has fetch";
+    ThreadingScenarioFailureStep = 1;
+    
+    // wait for the other thread to fetch too
+    while (ThreadingScenarioFailureStep < 2)
+    {
+      Orthanc::SystemToolbox::USleep(10000);
+    }
+    LOG(INFO) << "Thread1 will add after a short sleep";
+    Orthanc::SystemToolbox::USleep(100000);
+    LOG(INFO) << "Thread1 fails to add because of an error";
+  }
+}
+
+void ThreadingScenarioFailureThread2()
+{
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioFailureCache);
+  std::string v;
+
+  // wait until thread 1 has completed its "Fetch" but not added yet
+  while (ThreadingScenarioFailureStep < 1)
+  {
+    Orthanc::SystemToolbox::USleep(10000);
+  }
+
+  ThreadingScenarioFailureStep = 2;
+  LOG(INFO) << "Thread2 will fetch and wait for thread1 to add";
+  // this should wait until thread 1 has added
+  if (!a.Fetch(v, "key1"))
+  {
+    LOG(INFO) << "Thread2 has been awaken and will add since Thread1 has failed to add";
+    a.Add("key1", "value1");
+  }
+  LOG(INFO) << "Thread2 has added the value";
+}
+
+
+TEST(MemoryStringCache, ThreadingScenarioFailure)
+{
+  boost::thread thread1 = boost::thread(ThreadingScenarioFailureThread1);
+  boost::thread thread2 = boost::thread(ThreadingScenarioFailureThread2);
+
+  thread1.join();
+  thread2.join();
+}
+
+
+static int ThreadingScenarioInvalidateStep = 0;
+static Orthanc::MemoryStringCache ThreadingScenarioInvalidateCache;
+
+void ThreadingScenarioInvalidateThread1()
+{
+  // the first thread to call Fetch (will be in charge of adding)
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioInvalidateCache);
+  std::string v;
+
+  LOG(INFO) << "Thread1 will fetch";
+  if (!a.Fetch(v, "key1"))
+  {
+    LOG(INFO) << "Thread1 has fetch";
+    ThreadingScenarioInvalidateStep = 1;
+    
+    // wait for the other thread to fetch too
+    while (ThreadingScenarioInvalidateStep < 2)
+    {
+      Orthanc::SystemToolbox::USleep(10000);
+    }
+    LOG(INFO) << "Thread1 will invalidate after a short sleep";
+    Orthanc::SystemToolbox::USleep(100000);
+    LOG(INFO) << "Thread1 is invalidating";
+    ThreadingScenarioInvalidateCache.Invalidate("key1");
+  }
+}
+
+void ThreadingScenarioInvalidateThread2()
+{
+  Orthanc::MemoryStringCache::Accessor a(ThreadingScenarioInvalidateCache);
+  std::string v;
+
+  // wait until thread 1 has completed its "Fetch" but not added yet
+  while (ThreadingScenarioInvalidateStep < 1)
+  {
+    Orthanc::SystemToolbox::USleep(10000);
+  }
+
+  ThreadingScenarioInvalidateStep = 2;
+  LOG(INFO) << "Thread2 will fetch and wait for thread1 to add";
+  // this should wait until thread 1 has added
+  if (!a.Fetch(v, "key1"))
+  {
+    LOG(INFO) << "Thread2 has been awaken because thread1 has invalidated the key";
+  }
+}
+
+
+TEST(MemoryStringCache, ThreadingScenarioInvalidate)
+{
+  boost::thread thread1 = boost::thread(ThreadingScenarioInvalidateThread1);
+  boost::thread thread2 = boost::thread(ThreadingScenarioInvalidateThread2);
+
+  thread1.join();
+  thread2.join();
+}
--- a/OrthancServer/Sources/ServerContext.cpp	Thu Nov 09 08:51:01 2023 +0100
+++ b/OrthancServer/Sources/ServerContext.cpp	Mon Nov 13 17:01:59 2023 +0100
@@ -1078,7 +1078,7 @@
         
         {
           StorageAccessor accessor(area_, storageCache_, GetMetricsRegistry());
-          accessor.ReadStartRange(dicom, attachment.GetUuid(), FileContentType_Dicom, pixelDataOffset);
+          accessor.ReadStartRange(dicom, attachment, pixelDataOffset);
         }
         
         assert(dicom.size() == pixelDataOffset);
@@ -1219,7 +1219,7 @@
 
         StorageAccessor accessor(area_, storageCache_, GetMetricsRegistry());
 
-        accessor.ReadStartRange(dicom, attachment.GetUuid(), attachment.GetContentType(), pixelDataOffset);
+        accessor.ReadStartRange(dicom, attachment, pixelDataOffset);
         assert(dicom.size() == pixelDataOffset);
         
         return true;   // Success