Skip to content

Commit 44d14a7

Browse files
greateggsgreggreateggsgreg
committed
Thread-safety prep for free-threading builds
Two narrow fixes to remove obvious data races that already exist on the GIL build and become hot paths under Py_GIL_DISABLED: - InternString: replace plain Dictionary<> with ConcurrentDictionary<> for both _string2interns and _intern2strings. These are written from startup but read from every attribute-lookup hot path, so any concurrent shutdown/reinit could tear them. - ClassDerived.GetModuleBuilder: add a lock around the check-then-create on assemblyBuilders/moduleBuilders. The previous ContainsKey-then-DefineDynamicAssembly pattern had a TOCTOU race that could produce duplicate builders. Reset() also now locks for a clean reinitialisation. These are not sufficient for full free-threading support, but they remove low-hanging concurrency hazards.
1 parent dc69411 commit 44d14a7

2 files changed

Lines changed: 22 additions & 27 deletions

File tree

‎src/runtime/InternString.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.Diagnostics;
45
using System.Linq;
@@ -8,8 +9,8 @@ namespace Python.Runtime
89
{
910
static partial class InternString
1011
{
11-
private static readonly Dictionary<string, PyString> _string2interns = new();
12-
private static readonly Dictionary<IntPtr, string> _intern2strings = new();
12+
private static readonly ConcurrentDictionary<string, PyString> _string2interns = new();
13+
private static readonly ConcurrentDictionary<IntPtr, string> _intern2strings = new();
1314
const BindingFlags PyIdentifierFieldFlags = BindingFlags.Static | BindingFlags.NonPublic;
1415

1516
static InternString()
@@ -75,8 +76,8 @@ public static bool TryGetInterned(BorrowedReference op, out string s)
7576

7677
private static void SetIntern(string s, PyString op)
7778
{
78-
_string2interns.Add(s, op);
79-
_intern2strings.Add(op.Reference.DangerousGetAddress(), s);
79+
_string2interns.TryAdd(s, op);
80+
_intern2strings.TryAdd(op.Reference.DangerousGetAddress(), s);
8081
}
8182
}
8283
}

‎src/runtime/Types/ClassDerived.cs‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ internal class ClassDerivedObject : ClassObject
3333
{
3434
private static Dictionary<string, AssemblyBuilder> assemblyBuilders;
3535
private static Dictionary<Tuple<string, string>, ModuleBuilder> moduleBuilders;
36+
private static readonly object _buildersLock = new();
3637

3738
static ClassDerivedObject()
3839
{
@@ -42,8 +43,11 @@ static ClassDerivedObject()
4243

4344
public static void Reset()
4445
{
45-
assemblyBuilders = new Dictionary<string, AssemblyBuilder>();
46-
moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();
46+
lock (_buildersLock)
47+
{
48+
assemblyBuilders = new Dictionary<string, AssemblyBuilder>();
49+
moduleBuilders = new Dictionary<Tuple<string, string>, ModuleBuilder>();
50+
}
4751
}
4852

4953
internal ClassDerivedObject(Type tp) : base(tp)
@@ -657,33 +661,23 @@ private static void AddPythonProperty(string propertyName, PyObject func, TypeBu
657661

658662
private static ModuleBuilder GetModuleBuilder(string assemblyName, string moduleName)
659663
{
660-
// find or create a dynamic assembly and module
661-
AppDomain domain = AppDomain.CurrentDomain;
662-
ModuleBuilder moduleBuilder;
663-
664-
if (moduleBuilders.ContainsKey(Tuple.Create(assemblyName, moduleName)))
665-
{
666-
moduleBuilder = moduleBuilders[Tuple.Create(assemblyName, moduleName)];
667-
}
668-
else
664+
var key = Tuple.Create(assemblyName, moduleName);
665+
lock (_buildersLock)
669666
{
670-
AssemblyBuilder assemblyBuilder;
671-
if (assemblyBuilders.ContainsKey(assemblyName))
672-
{
673-
assemblyBuilder = assemblyBuilders[assemblyName];
674-
}
675-
else
667+
if (moduleBuilders.TryGetValue(key, out ModuleBuilder? existing))
668+
return existing;
669+
670+
if (!assemblyBuilders.TryGetValue(assemblyName, out AssemblyBuilder? assemblyBuilder))
676671
{
677-
assemblyBuilder = domain.DefineDynamicAssembly(new AssemblyName(assemblyName),
678-
AssemblyBuilderAccess.Run);
672+
assemblyBuilder = AppDomain.CurrentDomain.DefineDynamicAssembly(
673+
new AssemblyName(assemblyName), AssemblyBuilderAccess.Run);
679674
assemblyBuilders[assemblyName] = assemblyBuilder;
680675
}
681676

682-
moduleBuilder = assemblyBuilder.DefineDynamicModule(moduleName);
683-
moduleBuilders[Tuple.Create(assemblyName, moduleName)] = moduleBuilder;
677+
var moduleBuilder = assemblyBuilder.DefineDynamicModule(moduleName);
678+
moduleBuilders[key] = moduleBuilder;
679+
return moduleBuilder;
684680
}
685-
686-
return moduleBuilder;
687681
}
688682
}
689683

0 commit comments

Comments
 (0)