Skip to content

Commit 192f471

Browse files
committed
Updated provider lock with a simpler pattern
1 parent c075bdb commit 192f471

File tree

3 files changed

+31
-94
lines changed

3 files changed

+31
-94
lines changed

src/EventLogExpert.Eventing/EventResolvers/EventProviderDatabaseEventResolver.cs

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -73,73 +73,49 @@ public void ResolveProviderDetails(EventRecord eventRecord)
7373
ObjectDisposedException.ThrowIf(IsDisposed, nameof(EventProviderDatabaseEventResolver));
7474

7575
// Fast path: ConcurrentDictionary is thread-safe for reads, so we can check
76-
// without any lock. This avoids serializing all 8 parallel reader threads on
77-
// the UpgradeableReadLock for providers that are already cached.
76+
// without any lock. This avoids serializing all parallel reader threads for
77+
// providers that are already cached.
7878
if (ProviderDetails.ContainsKey(eventRecord.ProviderName)) { return; }
7979

80-
ProviderDetailsLock.EnterUpgradeableReadLock();
81-
82-
try
80+
using (ProviderDetailsLock.EnterScope())
8381
{
8482
// Re-check after acquiring lock - another thread may have added this provider
85-
if (ProviderDetails.ContainsKey(eventRecord.ProviderName))
86-
{
87-
return;
88-
}
83+
if (ProviderDetails.ContainsKey(eventRecord.ProviderName)) { return; }
8984

90-
ProviderDetailsLock.EnterWriteLock();
91-
92-
try
85+
// Use EnterScope() for exception-safe lock management
86+
using (_databaseAccessLock.EnterScope())
9387
{
94-
// Triple-check after acquiring write lock
95-
if (ProviderDetails.ContainsKey(eventRecord.ProviderName))
96-
{
97-
return;
98-
}
88+
// Check disposed inside the database lock to prevent race with Dispose().
89+
// This ensures we don't proceed if Dispose() has swapped out _dbContexts.
90+
ObjectDisposedException.ThrowIf(IsDisposed, nameof(EventProviderDatabaseEventResolver));
9991

100-
// Use EnterScope() for exception-safe lock management
101-
using (_databaseAccessLock.EnterScope())
92+
foreach (var dbContext in _dbContexts)
10293
{
103-
// Check disposed inside the database lock to prevent race with Dispose().
104-
// This ensures we don't proceed if Dispose() has swapped out _dbContexts.
105-
ObjectDisposedException.ThrowIf(IsDisposed, nameof(EventProviderDatabaseEventResolver));
106-
107-
foreach (var dbContext in _dbContexts)
108-
{
109-
// Use EF.Functions.Collate() with NOCASE on both sides for case-insensitive comparison.
110-
// Applying to both sides ensures the comparison is case-insensitive regardless of the
111-
// case in eventRecord.ProviderName. SQLite will use NOCASE collation for the comparison.
112-
// Note: The loop over databases is intentional for priority-based resolution.
113-
var details = dbContext.ProviderDetails.FirstOrDefault(p =>
114-
EF.Functions.Collate(p.ProviderName, "NOCASE") ==
115-
EF.Functions.Collate(eventRecord.ProviderName, "NOCASE"));
116-
117-
if (details is null) { continue; }
118-
119-
Logger?.Debug($"Resolved {eventRecord.ProviderName} provider from database {dbContext.Name}.");
120-
ProviderDetails.TryAdd(eventRecord.ProviderName, details);
121-
122-
// Exit after first match - databases are sorted by priority (SortDatabases),
123-
// so the first database containing the provider is the preferred source.
124-
// TryAdd would prevent duplicates anyway, but breaking early avoids unnecessary queries.
125-
break;
126-
}
94+
// Use EF.Functions.Collate() with NOCASE on both sides for case-insensitive comparison.
95+
// Applying to both sides ensures the comparison is case-insensitive regardless of the
96+
// case in eventRecord.ProviderName. SQLite will use NOCASE collation for the comparison.
97+
// Note: The loop over databases is intentional for priority-based resolution.
98+
var details = dbContext.ProviderDetails.FirstOrDefault(p =>
99+
EF.Functions.Collate(p.ProviderName, "NOCASE") ==
100+
EF.Functions.Collate(eventRecord.ProviderName, "NOCASE"));
101+
102+
if (details is null) { continue; }
103+
104+
Logger?.Debug($"Resolved {eventRecord.ProviderName} provider from database {dbContext.Name}.");
105+
ProviderDetails.TryAdd(eventRecord.ProviderName, details);
106+
107+
// Exit after first match - databases are sorted by priority (SortDatabases),
108+
// so the first database containing the provider is the preferred source.
109+
// TryAdd would prevent duplicates anyway, but breaking early avoids unnecessary queries.
110+
break;
127111
}
128112
}
129-
finally
130-
{
131-
ProviderDetailsLock.ExitWriteLock();
132-
}
133113

134114
if (!ProviderDetails.ContainsKey(eventRecord.ProviderName))
135115
{
136116
ProviderDetails.TryAdd(eventRecord.ProviderName, new ProviderDetails { ProviderName = eventRecord.ProviderName });
137117
}
138118
}
139-
finally
140-
{
141-
ProviderDetailsLock.ExitUpgradeableReadLock();
142-
}
143119
}
144120

145121
/// <summary>

src/EventLogExpert.Eventing/EventResolvers/EventResolverBase.cs

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public partial class EventResolverBase : IDisposable
4848
};
4949

5050
protected readonly ConcurrentDictionary<string, ProviderDetails?> ProviderDetails = new();
51-
protected readonly ReaderWriterLockSlim ProviderDetailsLock = new();
51+
protected readonly Lock ProviderDetailsLock = new();
5252

5353
private readonly IEventResolverCache? _cache;
5454
private readonly ITraceLogger? _logger;
@@ -113,27 +113,6 @@ protected virtual void Dispose(bool disposing)
113113
{
114114
return; // Already disposed by another thread
115115
}
116-
117-
// Attempt to dispose the lock to avoid leaking wait handles.
118-
// If other threads are still waiting to acquire the lock (rare race condition),
119-
// ReaderWriterLockSlim.Dispose() will throw SynchronizationLockException.
120-
// In that case, we suppress the exception to avoid crashing during disposal.
121-
// This is a pragmatic trade-off: accept a potential wait handle leak in an
122-
// extremely rare concurrent disposal scenario rather than crash the application.
123-
try
124-
{
125-
ProviderDetailsLock.Dispose();
126-
}
127-
catch (SynchronizationLockException)
128-
{
129-
// Lock is still being used by other threads. This is extremely rare but can happen
130-
// in concurrent disposal scenarios. We suppress the exception to prevent disposal
131-
// from throwing, accepting a potential wait handle leak as the lesser evil.
132-
}
133-
catch (ObjectDisposedException)
134-
{
135-
// Lock has already been disposed by another thread. Suppress to ensure disposal never throws.
136-
}
137116
}
138117

139118
private static void CleanupFormatting(ReadOnlySpan<char> unformattedString, ref Span<char> buffer, out int bufferIndex)

src/EventLogExpert.Eventing/EventResolvers/LocalProviderEventResolver.cs

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,13 @@ public void ResolveProviderDetails(EventRecord eventRecord)
2323
// Fast path: ConcurrentDictionary is thread-safe for reads
2424
if (ProviderDetails.ContainsKey(eventRecord.ProviderName)) { return; }
2525

26-
ProviderDetailsLock.EnterUpgradeableReadLock();
27-
28-
try
26+
using (ProviderDetailsLock.EnterScope())
2927
{
30-
// Re-check after acquiring lock
3128
if (ProviderDetails.ContainsKey(eventRecord.ProviderName)) { return; }
3229

33-
ProviderDetailsLock.EnterWriteLock();
34-
35-
try
36-
{
37-
if (ProviderDetails.ContainsKey(eventRecord.ProviderName)) { return; }
38-
39-
var details = new EventMessageProvider(eventRecord.ProviderName, Logger).LoadProviderDetails();
30+
var details = new EventMessageProvider(eventRecord.ProviderName, Logger).LoadProviderDetails();
4031

41-
ProviderDetails.TryAdd(eventRecord.ProviderName, details);
42-
}
43-
finally
44-
{
45-
ProviderDetailsLock.ExitWriteLock();
46-
}
47-
}
48-
finally
49-
{
50-
ProviderDetailsLock.ExitUpgradeableReadLock();
32+
ProviderDetails.TryAdd(eventRecord.ProviderName, details);
5133
}
5234
}
5335
}

0 commit comments

Comments
 (0)