Skip to content

Commit 9d3acdb

Browse files
oskarboskarb
committed
Fix race condition in how DefaultProxyAssemblyBuilder is used by ProxyFactory
(NH-3172), make cached proxy type lookup more efficient, and prevent race condition that could lead to creating the same proxy type multiple times.
1 parent eaea448 commit 9d3acdb

File tree

6 files changed

+41
-26
lines changed

6 files changed

+41
-26
lines changed

‎src/NHibernate.Test/DynamicProxyTests/PeVerifyFixture.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ public class ClassWithInternalConstructor
115115
public class SavingProxyAssemblyBuilder : IProxyAssemblyBuilder
116116
{
117117
private string assemblyName;
118-
private AssemblyBuilder assemblyBuilder;
119118

120119
public SavingProxyAssemblyBuilder(string assemblyName)
121120
{
@@ -125,16 +124,15 @@ public SavingProxyAssemblyBuilder(string assemblyName)
125124
public AssemblyBuilder DefineDynamicAssembly(AppDomain appDomain, AssemblyName name)
126125
{
127126
AssemblyBuilderAccess access = AssemblyBuilderAccess.RunAndSave;
128-
assemblyBuilder = appDomain.DefineDynamicAssembly(new AssemblyName(assemblyName), access);
129-
return assemblyBuilder;
127+
return appDomain.DefineDynamicAssembly(new AssemblyName(assemblyName), access);
130128
}
131129

132-
public ModuleBuilder DefineDynamicModule(string moduleName)
130+
public ModuleBuilder DefineDynamicModule(AssemblyBuilder assemblyBuilder, string moduleName)
133131
{
134132
return assemblyBuilder.DefineDynamicModule(moduleName, string.Format("{0}.mod", assemblyName), true);
135133
}
136134

137-
public void Save()
135+
public void Save(AssemblyBuilder assemblyBuilder)
138136
{
139137
assemblyBuilder.Save(assemblyName + ".dll");
140138
}

‎src/NHibernate/Proxy/DynamicProxy/DefaultProxyAssemblyBuilder.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,17 @@ namespace NHibernate.Proxy.DynamicProxy
66
{
77
public class DefaultProxyAssemblyBuilder : IProxyAssemblyBuilder
88
{
9-
private AssemblyBuilder assemblyBuilder;
10-
119
public AssemblyBuilder DefineDynamicAssembly(AppDomain appDomain, AssemblyName name)
1210
{
1311
#if DEBUG
1412
AssemblyBuilderAccess access = AssemblyBuilderAccess.RunAndSave;
1513
#else
1614
AssemblyBuilderAccess access = AssemblyBuilderAccess.Run;
1715
#endif
18-
assemblyBuilder = appDomain.DefineDynamicAssembly(name, access);
19-
return assemblyBuilder;
16+
return appDomain.DefineDynamicAssembly(name, access);
2017
}
2118

22-
public ModuleBuilder DefineDynamicModule(string moduleName)
19+
public ModuleBuilder DefineDynamicModule(AssemblyBuilder assemblyBuilder, string moduleName)
2320
{
2421
#if DEBUG
2522
ModuleBuilder moduleBuilder =
@@ -30,7 +27,7 @@ public ModuleBuilder DefineDynamicModule(string moduleName)
3027
return moduleBuilder;
3128
}
3229

33-
public void Save()
30+
public void Save(AssemblyBuilder assemblyBuilder)
3431
{
3532
#if DEBUG_PROXY_OUTPUT
3633
assemblyBuilder.Save("generatedAssembly.dll");

‎src/NHibernate/Proxy/DynamicProxy/IProxyAssemblyBuilder.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace NHibernate.Proxy.DynamicProxy
77
public interface IProxyAssemblyBuilder
88
{
99
AssemblyBuilder DefineDynamicAssembly(AppDomain appDomain, AssemblyName name);
10-
ModuleBuilder DefineDynamicModule(string moduleName);
11-
void Save();
10+
ModuleBuilder DefineDynamicModule(AssemblyBuilder assemblyBuilder, string moduleName);
11+
void Save(AssemblyBuilder assemblyBuilder);
1212
}
1313
}

‎src/NHibernate/Proxy/DynamicProxy/IProxyCache.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ public interface IProxyCache
1414
{
1515
bool Contains(System.Type baseType, params System.Type[] baseInterfaces);
1616
System.Type GetProxyType(System.Type baseType, params System.Type[] baseInterfaces);
17+
18+
bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out System.Type proxyType);
19+
1720
void StoreProxyType(System.Type result, System.Type baseType, params System.Type[] baseInterfaces);
1821
}
1922
}

‎src/NHibernate/Proxy/DynamicProxy/ProxyCache.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ public System.Type GetProxyType(System.Type baseType, params System.Type[] baseI
3434
return cache[entry];
3535
}
3636

37+
public bool TryGetProxyType(System.Type baseType, System.Type[] baseInterfaces, out System.Type proxyType)
38+
{
39+
proxyType = null;
40+
41+
if (baseType == null)
42+
return false;
43+
44+
var entry = new ProxyCacheEntry(baseType, baseInterfaces);
45+
return cache.TryGetValue(entry, out proxyType);
46+
}
47+
3748
public void StoreProxyType(System.Type result, System.Type baseType, params System.Type[] baseInterfaces)
3849
{
3950
var entry = new ProxyCacheEntry(baseType, baseInterfaces);

‎src/NHibernate/Proxy/DynamicProxy/ProxyFactory.cs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,21 +67,27 @@ public object CreateProxy(System.Type instanceType, IInterceptor interceptor, pa
6767
public System.Type CreateProxyType(System.Type baseType, params System.Type[] interfaces)
6868
{
6969
System.Type[] baseInterfaces = ReferenceEquals(null, interfaces) ? new System.Type[0] : interfaces.Where(t => t != null).ToArray();
70-
// Reuse the previous results, if possible
71-
if (Cache.Contains(baseType, baseInterfaces))
72-
{
73-
return Cache.GetProxyType(baseType, baseInterfaces);
74-
}
70+
71+
System.Type proxyType;
7572

76-
System.Type result = CreateUncachedProxyType(baseType, baseInterfaces);
73+
// Reuse the previous results, if possible, Fast path without locking.
74+
if (Cache.TryGetProxyType(baseType, baseInterfaces, out proxyType))
75+
return proxyType;
7776

78-
// Cache the proxy type
79-
if (result != null && Cache != null)
77+
lock (Cache)
8078
{
81-
Cache.StoreProxyType(result, baseType, baseInterfaces);
82-
}
79+
// Recheck in case we got interrupted.
80+
if (!Cache.TryGetProxyType(baseType, baseInterfaces, out proxyType))
81+
{
82+
proxyType = CreateUncachedProxyType(baseType, baseInterfaces);
8383

84-
return result;
84+
// Cache the proxy type
85+
if (proxyType != null && Cache != null)
86+
Cache.StoreProxyType(proxyType, baseType, baseInterfaces);
87+
}
88+
89+
return proxyType;
90+
}
8591
}
8692

8793
private System.Type CreateUncachedProxyType(System.Type baseType, System.Type[] baseInterfaces)
@@ -93,7 +99,7 @@ private System.Type CreateUncachedProxyType(System.Type baseType, System.Type[]
9399

94100
var name = new AssemblyName(assemblyName);
95101
AssemblyBuilder assemblyBuilder = ProxyAssemblyBuilder.DefineDynamicAssembly(currentDomain, name);
96-
ModuleBuilder moduleBuilder = ProxyAssemblyBuilder.DefineDynamicModule(moduleName);
102+
ModuleBuilder moduleBuilder = ProxyAssemblyBuilder.DefineDynamicModule(assemblyBuilder, moduleName);
97103

98104
TypeAttributes typeAttributes = TypeAttributes.AutoClass | TypeAttributes.Class |
99105
TypeAttributes.Public | TypeAttributes.BeforeFieldInit;
@@ -141,7 +147,7 @@ private System.Type CreateUncachedProxyType(System.Type baseType, System.Type[]
141147
AddSerializationSupport(baseType, baseInterfaces, typeBuilder, interceptorField, defaultConstructor);
142148
System.Type proxyType = typeBuilder.CreateType();
143149

144-
ProxyAssemblyBuilder.Save();
150+
ProxyAssemblyBuilder.Save(assemblyBuilder);
145151
return proxyType;
146152
}
147153

0 commit comments

Comments
 (0)