Update IncrementingPollingCounter only on Timer thread#105548
Update IncrementingPollingCounter only on Timer thread#105548noahfalk merged 3 commits intodotnet:mainfrom
Conversation
9c2156a to
fb98ef9
Compare
e3066f0 to
0837ec4
Compare
|
@dotnet-policy-service agree |
0837ec4 to
d871345
Compare
d871345 to
d0ecb01
Compare
|
@davmason, Hey! May I ask you too look at this? |
|
Added other people from dotnet-diag for review |
...ibraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs
Outdated
Show resolved
Hide resolved
...ibraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/IncrementingPollingCounter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/CounterGroup.cs
Show resolved
Hide resolved
d0ecb01 to
db5c90d
Compare
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/CounterGroup.cs
Outdated
Show resolved
Hide resolved
c05f914 to
649e4c6
Compare
noahfalk
left a comment
There was a problem hiding this comment.
This looks good to me - I suggested just a few minor tweaks.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/CounterGroup.cs
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/CounterGroup.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Noah Falk <[email protected]>
noahfalk
left a comment
There was a problem hiding this comment.
LGTM, thanks for your work on this @eterekhin!
|
@davmason - did you want to take another look? |
|
@noahfalk do we need a breaking change doc for this? if so, please file it through https://github.com/dotnet/docs/issues/new?assignees=gewarren&labels=breaking-change%2CPri1%2Cdoc-idea&projects=&template=02-breaking-change.yml&title=%5BBreaking+change%5D%3A+ and mark this PR with breaking change label? |
|
Would we typically write breaking change docs when we change an implementation choice that wasn't part of the original contract? I don't think of this as a breaking change in the formal sense but its true that some people may have taken dependencies on the threading behavior anyways. |
|
We usually document any change that can cause breaking to the user. If this change is possible to break any behavior, I suggest documenting it, but it is your call if you think otherwise. |
|
Cool, I'll file it. |
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/11224036440 |
|
/backport to release/8.0-staging |
|
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/11224091393 |

My idea is to call
_totalValueProvider()only on Timer thread . It should fix deadlocks like #93175 since the Timer thread doesn't holdEventListener.EventListenersLock