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.
This commit is contained in:
Ryan Francesconi
2026-04-04 08:52:54 -07:00
committed by GitHub
parent f32b503f56
commit 5d63187a8b

View File

@ -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;