From e17158fd797c3eae95fa4a1884b6e6f19e4b4e78 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:25:49 -0400 Subject: [PATCH 1/7] Fix TOCTOU race in XLiffBody.AddTransUnitRaw by moving check inside SpinLock (#150) Co-Authored-By: Claude Sonnet 4.6 --- src/L10NSharp/XLiffUtils/XLiffBody.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index 695b1f3..1620002 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -155,16 +155,19 @@ internal bool AddTransUnitRaw(XLiffTransUnit tu) tu.Id = (System.Threading.Interlocked.Increment(ref _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) From 2f67d0e43fac20a166da673210ae6fe4192aac91 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:31:31 -0400 Subject: [PATCH 2/7] Add changelog for TOCTOU fix in XLiffBody.AddTransUnitRaw (#150) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 895773a..ce46647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ 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) ### Removed From 9dbe0aed59811ad4dbd33a21fee8ddf1eb798587 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:40:09 -0400 Subject: [PATCH 3/7] Make _transUnitIdLock instance-level to avoid cross-instance contention --- src/L10NSharp/XLiffUtils/XLiffBody.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index 1620002..d9395d5 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(); From 57baecd267d3b898c6712032fa13c814c9080130 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:49:25 -0400 Subject: [PATCH 4/7] Replace redundant Interlocked.Increment with ++ inside SpinLock --- src/L10NSharp/XLiffUtils/XLiffBody.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index d9395d5..71d759f 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -152,7 +152,7 @@ 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; } From 0dc45b18176bac1ff8d0208d8c82f417c9f57faa Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:53:59 -0400 Subject: [PATCH 5/7] Document why RemoveTransUnit skips _transUnitIdLock --- src/L10NSharp/XLiffUtils/XLiffBody.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index 71d759f..d7b90fb 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -219,6 +219,7 @@ public void RemoveTransUnit(XLiffTransUnit tu) if (tu == null || tu.Id == null) return; + // No SpinLock needed: _transUnitIdLock guards ID assignment, not the dictionary itself. _transUnitDict.TryRemove(tu.Id, out _); TranslationsById.TryRemove(tu.Id, out _); } From eca6f104e71b9dce870c3cd81ec303e08e6d83a3 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 13:56:28 -0400 Subject: [PATCH 6/7] Add CHANGELOG entry for instance-level SpinLock change (#150) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce46647..21f8786 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [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) ### Removed From 18f906abb21400950cbb1234a05690ab88b3d43b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 8 Jun 2026 14:02:15 -0400 Subject: [PATCH 7/7] Fix misleading SpinLock comment in RemoveTransUnit --- src/L10NSharp/XLiffUtils/XLiffBody.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/L10NSharp/XLiffUtils/XLiffBody.cs b/src/L10NSharp/XLiffUtils/XLiffBody.cs index d7b90fb..c597a95 100644 --- a/src/L10NSharp/XLiffUtils/XLiffBody.cs +++ b/src/L10NSharp/XLiffUtils/XLiffBody.cs @@ -219,7 +219,8 @@ public void RemoveTransUnit(XLiffTransUnit tu) if (tu == null || tu.Id == null) return; - // No SpinLock needed: _transUnitIdLock guards ID assignment, not the dictionary itself. + // 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 _); }