Defuse throttled_func when it's accidentally engaged #18235
Conversation
lhecker
added
Issue-Bug
|
BTW there's a good chance that this bug was caused by a miscompilation. Lately I've had A LOT of crashes in random code which go away with a clean build. To be fair, I'm using the latest preview version of VS, but there may be some fun times coming for us. |
| decltype(_pendingRunArgs) args; | ||
| { | ||
| std::unique_lock guard{ _lock }; | ||
| args = std::exchange(_pendingRunArgs, std::nullopt); |
There was a problem hiding this comment.
As suggested by Dustin, this now uses std::exchange.
| { | ||
| _trailing_edge(); | ||
| } | ||
| _timer_callback(nullptr, this, nullptr); |
There was a problem hiding this comment.
Reading this code I've realized that I can ever so slightly reduce the binary size by moving _trailing_edge into the _timer_callback and calling _timer_callback directly.
There was a problem hiding this comment.
It depends honestly. If all of timer_callback is larger and less deduplicatable with COMDAT folding, it could be larger overall.
There was a problem hiding this comment.
Hm, I think I see what you mean. It does have the additional benefit that it swallows the exception, just like the regular timer callback. This makes it more predictable IMO, because it now behaves identical.
This change prevents
throttled_funcfrom reading uninitialized memoryunder some yet-unkown circumstances. The tl;dr is:
This simply moves the callback invocation into the storage.
That way we can centrally avoid invoking the callback accidentally.