From 5d63187a8b8d291cbf4f77d1bb973bc5ee91cf03 Mon Sep 17 00:00:00 2001
From: Ryan Francesconi <2917795+ryanfrancesconi@users.noreply.github.com>
Date: Sat, 4 Apr 2026 08:52:54 -0700
Subject: [PATCH] MP4: Fix data race in ItemFactory lazy map initialization
 (#1331)

Concurrent calls to propertyKeyForName() and handlerTypeForName() (e.g.
via batchMap during import) could race on the isEmpty() guard used for
first-call lazy initialization.

Replace isEmpty() guards with std::call_once / std::once_flag so that
each map is initialized exactly once in a thread-safe manner. Using
call_once (rather than eager construction in the base class constructor)
preserves virtual dispatch, allowing ItemFactory subclasses to override
nameHandlerMap() and namePropertyMap() correctly.

Both property maps are initialized together in a single once_flag since
nameForPropertyKey is derived from namePropertyMap.
---
 taglib/mp4/mp4itemfactory.cpp | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/taglib/mp4/mp4itemfactory.cpp b/taglib/mp4/mp4itemfactory.cpp
index 56aec794..9e591ad3 100644
--- a/taglib/mp4/mp4itemfactory.cpp
+++ b/taglib/mp4/mp4itemfactory.cpp
@@ -25,6 +25,7 @@
 
 #include "mp4itemfactory.h"
 
+#include <mutex>
 #include <utility>
 
 #include "tbytevector.h"
@@ -47,6 +48,8 @@ public:
   NameHandlerMap handlerTypeForName;
   Map<ByteVector, String> propertyKeyForName;
   Map<String, ByteVector> nameForPropertyKey;
+  mutable std::once_flag handlerMapOnce;
+  mutable std::once_flag propertyMapsOnce;
 };
 
 ItemFactory ItemFactory::factory;
@@ -239,9 +242,11 @@ std::pair<String, StringList> ItemFactory::itemToProperty(
 
 String ItemFactory::propertyKeyForName(const ByteVector &name) const
 {
-  if(d->propertyKeyForName.isEmpty()) {
+  std::call_once(d->propertyMapsOnce, [this] {
     d->propertyKeyForName = namePropertyMap();
-  }
+    for(const auto &[k, t] : std::as_const(d->propertyKeyForName))
+      d->nameForPropertyKey[t] = k;
+  });
   String key = d->propertyKeyForName.value(name);
   if(key.isEmpty() && name.startsWith(freeFormPrefix)) {
     key = name.mid(std::size(freeFormPrefix) - 1);
@@ -251,14 +256,11 @@ String ItemFactory::propertyKeyForName(const ByteVector &name) const
 
 ByteVector ItemFactory::nameForPropertyKey(const String &key) const
 {
-  if(d->nameForPropertyKey.isEmpty()) {
-    if(d->propertyKeyForName.isEmpty()) {
-      d->propertyKeyForName = namePropertyMap();
-    }
-    for(const auto &[k, t] : std::as_const(d->propertyKeyForName)) {
+  std::call_once(d->propertyMapsOnce, [this] {
+    d->propertyKeyForName = namePropertyMap();
+    for(const auto &[k, t] : std::as_const(d->propertyKeyForName))
       d->nameForPropertyKey[t] = k;
-    }
-  }
+  });
   ByteVector name = d->nameForPropertyKey.value(key);
   if(name.isEmpty() && !key.isEmpty()) {
     const auto &firstChar = key[0];
@@ -317,9 +319,9 @@ ItemFactory::NameHandlerMap ItemFactory::nameHandlerMap() const
 ItemFactory::ItemHandlerType ItemFactory::handlerTypeForName(
   const ByteVector &name) const
 {
-  if(d->handlerTypeForName.isEmpty()) {
+  std::call_once(d->handlerMapOnce, [this] {
     d->handlerTypeForName = nameHandlerMap();
-  }
+  });
   auto type = d->handlerTypeForName.value(name, ItemHandlerType::Unknown);
   if (type == ItemHandlerType::Unknown && name.size() == 4) {
     type = ItemHandlerType::Text;