OpenGLDevice: Lock pipeline cache on Linux

Prevents multiple processes from trampling on one another.

src/util/opengl_device.h

@@ -11,11 +11,18 @@
 #include "opengl_pipeline.h"
 #include "opengl_texture.h"
 
-#include <cstdio>
+#include "common/file_system.h"
+
 #include <memory>
 #include <string_view>
 #include <tuple>
 
+// Unix doesn't prevent concurrent write access, need to explicitly lock the pipeline cache.
+// Don't worry about Android, it's not like you can run one more than one instance of the app there...
+#if !defined(_WIN32) && !defined(__ANDROID__)
+#define OPENGL_PIPELINE_CACHE_NEEDS_LOCK 1
+#endif
+
 class OpenGLPipeline;
 class OpenGLStreamBuffer;
 class OpenGLTexture;
@@ -232,6 +239,9 @@ private:
   bool m_timestamp_query_started = false;
 
   std::FILE* m_pipeline_disk_cache_file = nullptr;
+#ifdef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
+  FileSystem::POSIXLock m_pipeline_disk_cache_file_lock;
+#endif
   u32 m_pipeline_disk_cache_data_end = 0;
   bool m_pipeline_disk_cache_changed = false;
 

src/util/opengl_pipeline.cpp

@@ -761,12 +761,22 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
 {
   DebugAssert(!m_pipeline_disk_cache_file);
 
-  std::FILE* fp = FileSystem::OpenCFile(path.c_str(), "r+b", error);
+  auto fp = FileSystem::OpenManagedCFile(path.c_str(), "r+b", error);
   if (!fp)
     return false;
 
+#ifdef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
+  // Unix doesn't prevent concurrent write access, need to explicitly lock it.
+  FileSystem::POSIXLock fp_lock(fp.get(), true, error);
+  if (!fp_lock.IsLocked())
+  {
+    Error::AddPrefix(error, "Failed to lock cache file: ");
+    return false;
+  }
+#endif
+
   // Read footer.
-  const s64 size = FileSystem::FSize64(fp);
+  const s64 size = FileSystem::FSize64(fp.get());
   if (size < static_cast<s64>(sizeof(PipelineDiskCacheFooter)) ||
       size >= static_cast<s64>(std::numeric_limits<u32>::max()))
   {
@@ -775,11 +785,10 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
   }
 
   PipelineDiskCacheFooter file_footer;
-  if (FileSystem::FSeek64(fp, size - sizeof(PipelineDiskCacheFooter), SEEK_SET) != 0 ||
+  if (FileSystem::FSeek64(fp.get(), size - sizeof(PipelineDiskCacheFooter), SEEK_SET) != 0 ||
-      std::fread(&file_footer, sizeof(file_footer), 1, fp) != 1)
+      std::fread(&file_footer, sizeof(file_footer), 1, fp.get()) != 1)
   {
     Error::SetStringView(error, "Invalid cache file footer.");
-    std::fclose(fp);
     return false;
   }
 
@@ -795,16 +804,15 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
         0)
   {
     Error::SetStringView(error, "Cache does not match expected driver/version.");
-    std::fclose(fp);
     return false;
   }
 
   m_pipeline_disk_cache_data_end = static_cast<u32>(size) - sizeof(PipelineDiskCacheFooter) -
                                    (sizeof(PipelineDiskCacheIndexEntry) * file_footer.num_programs);
-  if (m_pipeline_disk_cache_data_end < 0 || FileSystem::FSeek64(fp, m_pipeline_disk_cache_data_end, SEEK_SET) != 0)
+  if (m_pipeline_disk_cache_data_end < 0 ||
+      FileSystem::FSeek64(fp.get(), m_pipeline_disk_cache_data_end, SEEK_SET) != 0)
   {
     Error::SetStringView(error, "Failed to seek to start of index entries.");
-    std::fclose(fp);
     return false;
   }
 
@@ -812,12 +820,11 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
   for (u32 i = 0; i < file_footer.num_programs; i++)
   {
     PipelineDiskCacheIndexEntry entry;
-    if (std::fread(&entry, sizeof(entry), 1, fp) != 1 ||
+    if (std::fread(&entry, sizeof(entry), 1, fp.get()) != 1 ||
         (static_cast<s64>(entry.offset) + static_cast<s64>(entry.compressed_size)) >= size)
     {
       Error::SetStringView(error, "Failed to read disk cache entry.");
       m_program_cache.clear();
-      std::fclose(fp);
       return false;
     }
 
@@ -825,7 +832,6 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
     {
       Error::SetStringView(error, "Duplicate program in disk cache.");
       m_program_cache.clear();
-      std::fclose(fp);
       return false;
     }
 
@@ -840,15 +846,42 @@ bool OpenGLDevice::OpenPipelineCache(const std::string& path, Error* error)
   }
 
   VERBOSE_LOG("Read {} programs from disk cache.", m_program_cache.size());
-  m_pipeline_disk_cache_file = fp;
+  m_pipeline_disk_cache_file = fp.release();
+#ifdef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
+  m_pipeline_disk_cache_file_lock = std::move(fp_lock);
+#endif
   return true;
 }
 
 bool OpenGLDevice::CreatePipelineCache(const std::string& path, Error* error)
 {
+#ifndef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
   m_pipeline_disk_cache_file = FileSystem::OpenCFile(path.c_str(), "w+b", error);
   if (!m_pipeline_disk_cache_file)
     return false;
+#else
+  // Manually truncate it, that way we don't blow away another process's file on Linux.
+  m_pipeline_disk_cache_file = FileSystem::OpenCFile(path.c_str(), "a+b", error);
+  if (!m_pipeline_disk_cache_file || !FileSystem::FSeek64(m_pipeline_disk_cache_file, 0, SEEK_SET, error))
+    return false;
+
+  m_pipeline_disk_cache_file_lock = FileSystem::POSIXLock(m_pipeline_disk_cache_file, true, error);
+  if (!m_pipeline_disk_cache_file_lock.IsLocked())
+  {
+    Error::AddPrefix(error, "Failed to lock cache file: ");
+    std::fclose(m_pipeline_disk_cache_file);
+    m_pipeline_disk_cache_file = nullptr;
+    return false;
+  }
+
+  if (!FileSystem::FTruncate64(m_pipeline_disk_cache_file, 0, error))
+  {
+    Error::AddPrefix(error, "Failed to truncate cache file: ");
+    m_pipeline_disk_cache_file_lock = {};
+    std::fclose(m_pipeline_disk_cache_file);
+    m_pipeline_disk_cache_file = nullptr;
+  }
+#endif
 
   m_pipeline_disk_cache_data_end = 0;
   m_pipeline_disk_cache_changed = true;
@@ -982,6 +1015,9 @@ bool OpenGLDevice::DiscardPipelineCache()
   if (!FileSystem::FTruncate64(m_pipeline_disk_cache_file, 0, &error))
   {
     ERROR_LOG("Failed to truncate pipeline cache: {}", error.GetDescription());
+#ifdef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
+    m_pipeline_disk_cache_file_lock.Unlock();
+#endif
     std::fclose(m_pipeline_disk_cache_file);
     m_pipeline_disk_cache_file = nullptr;
     return false;
@@ -994,19 +1030,25 @@ bool OpenGLDevice::DiscardPipelineCache()
 
 bool OpenGLDevice::ClosePipelineCache(const std::string& filename, Error* error)
 {
+  const auto close_cache = [this]() {
+#ifdef OPENGL_PIPELINE_CACHE_NEEDS_LOCK
+    m_pipeline_disk_cache_file_lock.Unlock();
+#endif
+    std::fclose(m_pipeline_disk_cache_file);
+    m_pipeline_disk_cache_file = nullptr;
+  };
+
   if (!m_pipeline_disk_cache_changed)
   {
     VERBOSE_LOG("Not updating pipeline cache because it has not changed.");
-    std::fclose(m_pipeline_disk_cache_file);
+    close_cache();
-    m_pipeline_disk_cache_file = nullptr;
     return true;
   }
 
   // Rewrite footer/index entries.
   if (!FileSystem::FSeek64(m_pipeline_disk_cache_file, m_pipeline_disk_cache_data_end, SEEK_SET, error) != 0)
   {
-    std::fclose(m_pipeline_disk_cache_file);
+    close_cache();
-    m_pipeline_disk_cache_file = nullptr;
     return false;
   }
 
@@ -1027,8 +1069,7 @@ bool OpenGLDevice::ClosePipelineCache(const std::string& filename, Error* error)
     if (std::fwrite(&entry, sizeof(entry), 1, m_pipeline_disk_cache_file) != 1) [[unlikely]]
     {
       Error::SetErrno(error, "fwrite() for entry failed: ", errno);
-      std::fclose(m_pipeline_disk_cache_file);
+      close_cache();
-      m_pipeline_disk_cache_file = nullptr;
       return false;
     }
 
@@ -1039,15 +1080,14 @@ bool OpenGLDevice::ClosePipelineCache(const std::string& filename, Error* error)
   FillFooter(&footer, m_shader_cache.GetVersion());
   footer.num_programs = count;
 
-  if (std::fwrite(&footer, sizeof(footer), 1, m_pipeline_disk_cache_file) != 1) [[unlikely]]
+  if (std::fwrite(&footer, sizeof(footer), 1, m_pipeline_disk_cache_file) != 1 ||
+      std::fflush(m_pipeline_disk_cache_file) != 0) [[unlikely]]
   {
     Error::SetErrno(error, "fwrite() for footer failed: ", errno);
-    std::fclose(m_pipeline_disk_cache_file);
+    close_cache();
-    m_pipeline_disk_cache_file = nullptr;
+    return false;
   }
 
-  if (std::fclose(m_pipeline_disk_cache_file) != 0)
+  close_cache();
-    Error::SetErrno(error, "fclose() failed: ", errno);
-  m_pipeline_disk_cache_file = nullptr;
   return true;
 }