fixes xmp thread safety

Exiv2 · Jul 8, 2024 · d71229e · d71229e
1 parent 35a6b8f
commit d71229e
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 16 deletions.
diff --git a/include/exiv2/xmp_exiv2.hpp b/include/exiv2/xmp_exiv2.hpp
@@ -4,6 +4,7 @@
 #define XMP_HPP_
 
 // *****************************************************************************
+#include <atomic>
 #include "exiv2lib_export.h"
 
 // included header files
@@ -355,7 +356,9 @@ class EXIV2API XmpParser {
 
     @return True if the initialization was successful, else false.
    */
-  static bool initialize(XmpParser::XmpLockFct xmpLockFct = nullptr, void* pLockData = nullptr);
+  [[deprecated("xmpLockFct is no longer required")]] static bool initialize(XmpParser::XmpLockFct xmpLockFct, void* pLockData);
+
+  static bool initialize();
   /*!
     @brief Terminate the XMP Toolkit and unregister custom namespaces.
 
@@ -365,6 +368,9 @@ class EXIV2API XmpParser {
   static void terminate();
 
  private:
+
+  static bool initialize_unsafe();
+
   /*!
     @brief Register a namespace with the XMP Toolkit.
    */
@@ -382,9 +388,8 @@ class EXIV2API XmpParser {
   static void registeredNamespaces(Exiv2::Dictionary&);
 
   // DATA
-  static bool initialized_;  //! Indicates if the XMP Toolkit has been initialized
-  static XmpLockFct xmpLockFct_;
-  static void* pLockData_;
+  static std::atomic_bool initialized_;  //! Indicates if the XMP Toolkit has been initialized
+  static std::mutex global_mutex_; // meant to synchronize all xmp actions across all threads
 
   friend class XmpProperties;  // permit XmpProperties -> registerNs() and registeredNamespaces()
 

diff --git a/src/xmp.cpp b/src/xmp.cpp
@@ -504,15 +504,12 @@ void XmpData::eraseFamily(XmpData::iterator& pos) {
   }
 }
 
-bool XmpParser::initialized_ = false;
-XmpParser::XmpLockFct XmpParser::xmpLockFct_ = nullptr;
-void* XmpParser::pLockData_ = nullptr;
+std::atomic_bool XmpParser::initialized_ = false;
+std::mutex XmpParser::global_mutex_;
 
 #ifdef EXV_HAVE_XMP_TOOLKIT
-bool XmpParser::initialize(XmpParser::XmpLockFct xmpLockFct, void* pLockData) {
+bool XmpParser::initialize_unsafe() {
   if (!initialized_) {
-    xmpLockFct_ = xmpLockFct;
-    pLockData_ = pLockData;
     initialized_ = SXMPMeta::Initialize();
 #ifdef EXV_ADOBE_XMPSDK
     SXMPMeta::RegisterNamespace("http://ns.adobe.com/lightroom/1.0/", "lr", nullptr);
@@ -565,12 +562,21 @@ bool XmpParser::initialize(XmpParser::XmpLockFct xmpLockFct, void* pLockData) {
   return initialized_;
 }
 #else
-bool XmpParser::initialize(XmpParser::XmpLockFct, void*) {
+bool XmpParser::initialize_unsafe(XmpParser::XmpLockFct, void*) {
   initialized_ = true;
   return initialized_;
 }
 #endif
 
+bool XmpParser::initialize([[maybe_unused]] XmpParser::XmpLockFct xmpLockFct, [[maybe_unused]] void* pLockData) {
+  return initialize();
+}
+
+bool XmpParser::initialize() {
+  auto scopedWriteLock = std::scoped_lock(XmpParser::global_mutex_);
+  return initialize_unsafe();
+}
+
 #ifdef EXV_HAVE_XMP_TOOLKIT
 static XMP_Status nsDumper(void* refCon, XMP_StringPtr buffer, XMP_StringLen bufferSize) {
   XMP_Status result = 0;
@@ -607,7 +613,8 @@ void XmpParser::registeredNamespaces(Exiv2::Dictionary& dict) {
   bool bInit = !initialized_;
   try {
     if (bInit)
-      initialize();
+      bInit = initialize_unsafe();
+    auto scopedWriteLock = std::scoped_lock(XmpParser::global_mutex_);
     SXMPMeta::DumpNamespaces(nsDumper, &dict);
     if (bInit)
       terminate();
@@ -633,8 +640,7 @@ void XmpParser::terminate() {
 #ifdef EXV_HAVE_XMP_TOOLKIT
 void XmpParser::registerNs(const std::string& ns, const std::string& prefix) {
   try {
-    initialize();
-    AutoLock autoLock(xmpLockFct_, pLockData_);
+    initialize_unsafe();
     SXMPMeta::DeleteNamespace(ns.c_str());
 #ifdef EXV_ADOBE_XMPSDK
     SXMPMeta::RegisterNamespace(ns.c_str(), prefix.c_str(), nullptr);
@@ -665,12 +671,14 @@ void XmpParser::unregisterNs(const std::string& /*ns*/) {
 #ifdef EXV_HAVE_XMP_TOOLKIT
 int XmpParser::decode(XmpData& xmpData, const std::string& xmpPacket) {
   try {
+    auto scopedWriteLock = std::scoped_lock(XmpParser::global_mutex_);
+
     xmpData.clear();
     xmpData.setPacket(xmpPacket);
     if (xmpPacket.empty())
       return 0;
 
-    if (!initialize()) {
+    if (!initialize_unsafe()) {
 #ifndef SUPPRESS_WARNINGS
       EXV_ERROR << "XMP toolkit initialization failed.\n";
 #endif
@@ -809,12 +817,14 @@ int XmpParser::decode(XmpData& xmpData, const std::string& xmpPacket) {
 #ifdef EXV_HAVE_XMP_TOOLKIT
 int XmpParser::encode(std::string& xmpPacket, const XmpData& xmpData, uint16_t formatFlags, uint32_t padding) {
   try {
+    auto scopedWriteLock = std::scoped_lock(XmpParser::global_mutex_);
+
     if (xmpData.empty()) {
       xmpPacket.clear();
       return 0;
     }
 
-    if (!initialize()) {
+    if (!initialize_unsafe()) {
 #ifndef SUPPRESS_WARNINGS
       EXV_ERROR << "XMP toolkit initialization failed.\n";
 #endif