Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582

Merged
VSadov merged 19 commits into
mainfrom
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged

Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom
copilot/fix-noop-limiter-disposal

Conversation

CopilotAI commented Apr 29, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.

Changes Made

  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.

CopilotAI requested review from Copilot and removed request for CopilotApril 29, 2026 20:47
CopilotAI linked an issue Apr 29, 2026 that may be closed by this pull request
DefaultPartitionedRateLimiter never disposes NoopLimiter #124557
Closed
@dotnet-policy-service

dotnet-policy-serviceBot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

CopilotAI requested review from Copilot and removed request for CopilotApril 29, 2026 21:12
CopilotAI changed the title [WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI requested a review from VSadovApril 29, 2026 21:18
CopilotAI requested review from Copilot and removed request for CopilotApril 29, 2026 22:57
CopilotAI requested a review from VSadovApril 29, 2026 22:58
CopilotAI requested review from Copilot and removed request for CopilotApril 29, 2026 23:19
This was referenced Apr 30, 2026
Open
Open
CopilotAI removed the request for review from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot mentioned this pull request Jun 14, 2026
Closed
eduardo-vp
eduardo-vp approved these changes Jun 16, 2026
View reviewed changes

@eduardo-vpeduardo-vp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov

VSadov commented Jun 16, 2026

Copy link
Copy Markdown
Member

Thanks!

CopilotAIand others added 17 commits June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI reviewed Jun 17, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot mentioned this pull request Jun 17, 2026
Closed
@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI reviewed Jun 17, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs Outdated
@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI reviewed Jun 18, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot mentioned this pull request Jun 19, 2026
Merged
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov

Copilot code reviewCopilot

Labels

area-System.Threading

Projects

None yet

Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter

5 participants

@kg@VSadov@eduardo-vp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat - #127582
Merged
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Jun 18, 2026
Merged
Fix NoopLimiter disposal in DefaultPartitionedRateLimiter Heartbeat#127582
VSadov merged 19 commits into
mainfrom 
copilot/fix-noop-limiter-disposal
Conversation
CopilotAI
commented
Apr 29, 2026•

edited

Loading

Copy link


Copy Markdown

Contributor
Fixes DefaultPartitionedRateLimiter so that NoopLimiter partitions are evicted and disposed by the Heartbeat timer, which previously never happened because NoopLimiter.IdleDuration always returns null.
Changes Made
  • Introduced a LimiterEntry wrapper that tracks a LastAccessTimestamp (updated on each Acquire/WaitAsync call) alongside the RateLimiter instance.
  • Added GetIdleDuration(LimiterEntry) helper that falls back to the elapsed time since last access for NoopLimiter (whose IdleDuration is always null), and returns null for all other limiter types that return null (preserving the existing "do not evict" contract).
  • Updated the Heartbeat eviction check to use the is TimeSpan idleDuration && idleDuration > s_idleTimeLimit pattern (with ?? TimeSpan.Zero for the under-lock re-check), correctly handling the nullable TimeSpan? return — null skips eviction, matching the original pattern.
  • Used Volatile.Read/Volatile.Write for atomic 64-bit timestamp access on 32-bit platforms.
  • Added a RateLimiterHelper.GetElapsedTime(long) overload to avoid .GetValueOrDefault() at call sites.
  • Added tests covering NoopLimiter eviction via Heartbeat and verifying that non-NoopLimiter partitions with IdleDuration == null are not evicted while active.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 20:47
CopilotAI
linked an issue
Apr 29, 2026
that may be
closed
by this pull request


DefaultPartitionedRateLimiter never disposes NoopLimiter
#124557

Closed

@dotnet-policy-service
dotnet-policy-serviceBot
commented
Apr 29, 2026

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov

See info in area-owners.md if you want to be subscribed.
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 21:12
CopilotAI
changed the title
[WIP] Fix DefaultPartitionedRateLimiter to dispose NoopLimiterEvict idle partitions whose IdleDuration is always nullApr 29, 2026
CopilotAI
requested a review
from VSadovApril 29, 2026 21:18
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 22:57
CopilotAI
requested a review
from VSadovApril 29, 2026 22:58
CopilotAI
requested review from
Copilot
and removed request for
CopilotApril 29, 2026 23:19

This was referenced Apr 30, 2026

Open


Open

CopilotAI
removed the request for review
from CopilotJune 14, 2026 18:31
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 14, 2026

Closed

eduardo-vp
eduardo-vp
approved these changes
Jun 16, 2026
View reviewed changes
@eduardo-vpeduardo-vp
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Comment thread...braries/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimiterHelper.cs
@VSadov
VSadov
commented
Jun 16, 2026

Copy link


Copy Markdown

Member
Thanks!
CopilotAIand others
added 17 commits
June 16, 2026 22:02
@VSadov
Track per-partition last-access tick so NoopLimiter is evicted by Hea…
185affb
…rtbeat
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Address review: remove redundant timestamp write, use Interlocked for…
4b31962
… atomic 64-bit access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify comment explaining why first-access has no eviction window
8d250d5
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7cfc3a4e-ebbd-4056-97a5-69edc299af17
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use Volatile.Read/Write instead of Interlocked for LastAccessTimestamp
c7e37cd
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5d38c664-8bf2-4a12-a8d4-ddff0d031dbd
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add RateLimiterHelper.GetElapsedTime(long) overload to avoid GetValue…
647bd0c
…OrDefault
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/28a47963-2d4b-442b-933c-ddc94f1f5389
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Restrict idle eviction fallback to NoopLimiter
2d2f3ed
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Apply remaining changes
762a5ac
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Merge main + fix build after Utils.RunTimerFunc signature change
9e0efe9
Merges current main into the PR and updates the two new test methods (NoopLimiterPartitionIsCleanedUp and LimiterWithNullIdleDurationIsNotCleanedUp) to:
- Pass explicit type arguments to Utils.RunTimerFunc<TResource, TKey> (signature changed in main).
- Use the trim-safe Type.GetType + MakeGenericType pattern in the reflection helpers (BackdateLastAccessTimestamp / GetLimiterEntry / GetLazyLimiterEntry) now that the tests project no longer disables the trim analyzer.
All 344 tests pass; net11.0 and net481 build with 0 warnings.
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Remove unrelated scratch files accidentally committed earlier
a6443a9
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
0119d3d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Potential fix for pull request finding
cc19731
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VSadov
Encapsulate limiter entry timestamp
3dec367
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Centralize reflected type names in rate limiting tests
65a161d
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add Assert.True(limiterType.IsInstanceOfType(limiter)) before GetValu…
c621259
…e in GetLazyLimiterEntry
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Add explicit null check for nullable TimeSpan comparison in Heartbeat
56eadb6
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Clarify RateLimiting test comment
97da373
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
@VSadov
Use is-pattern for nullable TimeSpan comparison in Heartbeat eviction
72f0a07
Co-authored-by: VSadov <8218165+VSadov@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comment thread...em.Threading.RateLimiting/src/System/Threading/RateLimiting/DefaultPartitionedRateLimiter.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 17, 2026

Closed

@VSadov
Potential fix for pull request finding
7e6354d
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 17, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Outdated

@VSadov
Potential fix for pull request finding
ff41bbc
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI
reviewed
Jun 18, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
Comment threadsrc/libraries/System.Threading.RateLimiting/tests/PartitionedRateLimiterTests.cs
@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 19, 2026
 Merged

Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eduardo-vpeduardo-vpeduardo-vp approved these changes
@VSadovVSadovAwaiting requested review from VSadov

+1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@VSadovVSadov
Copilot code reviewCopilot

Labels

area-System.Threading

Projects


None yet


Milestone

11.0-preview6

Development

Successfully merging this pull request may close these issues.

DefaultPartitionedRateLimiter never disposes NoopLimiter


5 participants

@kg@VSadov@eduardo-vp