diff --git a/CHANGELOG.md b/CHANGELOG.md index ba0ced9..bca5b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [L10NSharp.Windows.Forms] Restored project-local Resources support for `FallbackLanguagesDlgBase` button images (`Move`, `Move_up`, and `Move_down`). - [L10NSharp.Windows.Forms] Corrected resource manager base name to `L10NSharp.Windows.Forms.Properties.Resources`. - [L10NSharp.Windows.Forms.Tests] Corrected resource manager base name to `L10NSharp.Windows.Forms.Tests.Properties.Resources`. +- [L10NSharp] Fixed race condition in `XLiffBody.AddTransUnitRaw` where two concurrent threads with the same translation-unit ID could both bypass the duplicate check and silently overwrite each other's entry. (#150) +- [L10NSharp] Eliminated unnecessary cross-instance lock contention in `XLiffBody` by making `_transUnitIdLock` instance-level instead of static. (#150) - [L10NSharp] Fixed `FilenamesToAddToCache` yielding both the custom and installed XLIFF for the same language when `UseLanguageCodeFolders` is `true`, causing custom translations to be silently overwritten by installed ones. (#140) - [L10NSharp] Fixed `ExtractXliff` accumulating duplicate "Not found in static scan" notes on successive runs; the note is now replaced rather than appended, and removed when the string is subsequently found. (#113) - [L10NSharp] Fixed `LocalizationManager.GetString` silently falling back to English when called with a one-shot `IEnumerable` for `preferredLanguageIds`; the sequence is now materialized before use. (#139) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index 695b1f3..c597a95 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -25,7 +25,7 @@ public class XLiffBody // This is used when translation unit IDs are not found in the file (which seems to be // the case with Lingobit XLiff files). private int _transUnitId; - static SpinLock _transUnitIdLock; + SpinLock _transUnitIdLock; private readonly ConcurrentDictionary _transUnitDict = new ConcurrentDictionary(); @@ -152,19 +152,22 @@ internal bool AddTransUnitRaw(XLiffTransUnit tu) key = tu.Id; if (string.IsNullOrEmpty(key)) { - tu.Id = (System.Threading.Interlocked.Increment(ref _transUnitId)).ToString(); + tu.Id = (++_transUnitId).ToString(); key = tu.Id; } + + // If a translation unit with the specified id already exists, then quit here. + // This check and the dictionary write must both happen inside the lock to avoid + // a TOCTOU race where two threads with the same ID both pass the check. + if (GetTransUnitForId(key) != null) + return false; + _transUnitDict[key] = tu; } finally { if (lockTaken) _transUnitIdLock.Exit(false); } - // If a translation unit with the specified id already exists, then quit here. - if (GetTransUnitForId(tu.Id) != null) - return false; - _transUnitDict[key] = tu; return true; } public bool AddTransUnit(XLiffTransUnit tu) @@ -216,6 +219,8 @@ public void RemoveTransUnit(XLiffTransUnit tu) if (tu == null || tu.Id == null) return; + // No SpinLock needed: _transUnitIdLock serializes AddTransUnitRaw's check-then-add + // sequence; TryRemove is independently atomic. _transUnitDict.TryRemove(tu.Id, out _); TranslationsById.TryRemove(tu.Id, out _); }