Skip to content

Commit daa130d

Browse files
committed
Improve rate limiter and connection disposal logic
Refactored InMemoryRateLimiter's RequestWindow to use primary constructor and simplified field initialization. Enhanced ConnectionTracker disposal logic to handle race conditions by re-adding connections if new activity is detected, and added object disposal checks for lock methods. Introduced MarkAsActiveUnsafe to reset removal status under lock.
1 parent 8c91df0 commit daa130d

2 files changed

Lines changed: 64 additions & 35 deletions

File tree

src/Zetian/Extensions/RateLimiting/InMemoryRateLimiter.cs

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -112,44 +112,35 @@ public void Dispose()
112112
_windows.Clear();
113113
}
114114

115-
private class RequestWindow
115+
private class RequestWindow(RateLimitConfiguration configuration)
116116
{
117-
private readonly RateLimitConfiguration _configuration;
118117
private readonly object _lock = new();
119-
private readonly Queue<DateTime> _requests;
120-
private DateTime _windowStart;
121-
private int _requestCount;
122-
123-
public RequestWindow(RateLimitConfiguration configuration)
124-
{
125-
_configuration = configuration;
126-
_requests = new Queue<DateTime>();
127-
_windowStart = DateTime.UtcNow;
128-
_requestCount = 0;
129-
}
118+
private readonly Queue<DateTime> _requests = new();
119+
private DateTime _windowStart = DateTime.UtcNow;
120+
private int _requestCount = 0;
130121

131122
public bool IsAllowed()
132123
{
133124
lock (_lock)
134125
{
135126
CleanupOldRequests();
136127

137-
if (_configuration.UseSlidingWindow)
128+
if (configuration.UseSlidingWindow)
138129
{
139-
return _requests.Count < _configuration.MaxRequests;
130+
return _requests.Count < configuration.MaxRequests;
140131
}
141132
else
142133
{
143134
// Fixed window
144135
DateTime now = DateTime.UtcNow;
145-
if (now - _windowStart > _configuration.Window)
136+
if (now - _windowStart > configuration.Window)
146137
{
147138
// New window
148139
_windowStart = now;
149140
_requestCount = 0;
150141
}
151142

152-
return _requestCount < _configuration.MaxRequests;
143+
return _requestCount < configuration.MaxRequests;
153144
}
154145
}
155146
}
@@ -160,15 +151,15 @@ public void RecordRequest()
160151
{
161152
DateTime now = DateTime.UtcNow;
162153

163-
if (_configuration.UseSlidingWindow)
154+
if (configuration.UseSlidingWindow)
164155
{
165156
CleanupOldRequests();
166157
_requests.Enqueue(now);
167158
}
168159
else
169160
{
170161
// Fixed window
171-
if (now - _windowStart > _configuration.Window)
162+
if (now - _windowStart > configuration.Window)
172163
{
173164
// New window
174165
_windowStart = now;
@@ -188,19 +179,19 @@ public int GetRemaining()
188179
{
189180
CleanupOldRequests();
190181

191-
if (_configuration.UseSlidingWindow)
182+
if (configuration.UseSlidingWindow)
192183
{
193-
return Math.Max(0, _configuration.MaxRequests - _requests.Count);
184+
return Math.Max(0, configuration.MaxRequests - _requests.Count);
194185
}
195186
else
196187
{
197188
DateTime now = DateTime.UtcNow;
198-
if (now - _windowStart > _configuration.Window)
189+
if (now - _windowStart > configuration.Window)
199190
{
200-
return _configuration.MaxRequests;
191+
return configuration.MaxRequests;
201192
}
202193

203-
return Math.Max(0, _configuration.MaxRequests - _requestCount);
194+
return Math.Max(0, configuration.MaxRequests - _requestCount);
204195
}
205196
}
206197
}
@@ -211,27 +202,27 @@ public bool IsExpired()
211202
{
212203
DateTime now = DateTime.UtcNow;
213204

214-
if (_configuration.UseSlidingWindow)
205+
if (configuration.UseSlidingWindow)
215206
{
216207
CleanupOldRequests();
217208
return _requests.Count == 0 &&
218-
(_requests.Count == 0 || now - _windowStart > _configuration.Window * 2);
209+
(_requests.Count == 0 || now - _windowStart > configuration.Window * 2);
219210
}
220211
else
221212
{
222-
return now - _windowStart > _configuration.Window * 2;
213+
return now - _windowStart > configuration.Window * 2;
223214
}
224215
}
225216
}
226217

227218
private void CleanupOldRequests()
228219
{
229-
if (!_configuration.UseSlidingWindow)
220+
if (!configuration.UseSlidingWindow)
230221
{
231222
return;
232223
}
233224

234-
DateTime cutoff = DateTime.UtcNow - _configuration.Window;
225+
DateTime cutoff = DateTime.UtcNow - configuration.Window;
235226

236227
while (_requests.Count > 0 && _requests.Peek() < cutoff)
237228
{

src/Zetian/Internal/ConnectionTracker.cs

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,18 +204,32 @@ private void CleanupExpired(object? state)
204204
{
205205
if (_connections.TryRemove(key, out ConnectionInfo? removed))
206206
{
207-
// Only dispose if still marked for removal
208-
removed.Lock();
207+
// Double-check under lock before disposing
209208
try
210209
{
211-
if (removed.IsMarkedForRemovalUnsafe())
210+
removed.Lock();
211+
try
212212
{
213-
removed.Dispose();
213+
// Re-verify no new connections were acquired
214+
if (removed.IsMarkedForRemovalUnsafe() && removed.GetCountUnsafe() == 0)
215+
{
216+
removed.Dispose();
217+
}
218+
else
219+
{
220+
// Race condition: new connection acquired, put it back
221+
removed.MarkAsActiveUnsafe();
222+
_connections.TryAdd(key, removed);
223+
}
224+
}
225+
finally
226+
{
227+
removed.ReleaseLock();
214228
}
215229
}
216-
finally
230+
catch (ObjectDisposedException)
217231
{
218-
removed.ReleaseLock();
232+
// Already disposed by another thread, ignore
219233
}
220234
}
221235
}
@@ -280,6 +294,14 @@ internal sealed class ConnectionInfo(int maxConnections) : IDisposable
280294
/// </summary>
281295
public Task LockAsync(CancellationToken cancellationToken = default)
282296
{
297+
#if NET6_0
298+
if (_disposed)
299+
{
300+
throw new ObjectDisposedException(nameof(ConnectionInfo));
301+
}
302+
#elif NET7_0_OR_GREATER
303+
ObjectDisposedException.ThrowIf(_disposed, nameof(ConnectionInfo));
304+
#endif
283305
return _syncLock.WaitAsync(cancellationToken);
284306
}
285307

@@ -288,6 +310,14 @@ public Task LockAsync(CancellationToken cancellationToken = default)
288310
/// </summary>
289311
public void Lock()
290312
{
313+
#if NET6_0
314+
if (_disposed)
315+
{
316+
throw new ObjectDisposedException(nameof(ConnectionInfo));
317+
}
318+
#elif NET7_0_OR_GREATER
319+
ObjectDisposedException.ThrowIf(_disposed, nameof(ConnectionInfo));
320+
#endif
291321
_syncLock.Wait();
292322
}
293323

@@ -362,6 +392,14 @@ public bool IsMarkedForRemovalUnsafe()
362392
return _markedForRemoval;
363393
}
364394

395+
/// <summary>
396+
/// Marks as active again (must be called under lock)
397+
/// </summary>
398+
public void MarkAsActiveUnsafe()
399+
{
400+
_markedForRemoval = false;
401+
}
402+
365403
/// <summary>
366404
/// Releases all resources used by the current instance.
367405
/// </summary>

0 commit comments

Comments
 (0)