Fail FuncEval if slot backpatching lock is held by any thread #2380

kouvel · 2020-01-30T09:09:24Z

In many cases cooperative GC mode is entered after acquiring the slot backpatching lock and the thread may block for debugger suspension while holding the lock. A FuncEval may time out on entering the lock if for example it calls a virtual or interface method for the first time. Failing the FuncEval when the lock is held enables the debugger to fall back to other options for expression evaluation.
Also added polls for debugger suspension before acquiring the slot backpatching lock on background threads that often operate in preemptive GC mode. A common case is when the debugger breaks while the tiering delay timer is active, the timer ticks shortly afterwards (after debugger suspension completes) and if a thread pool thread is already available, the background thread would block while holding the lock. The poll checks for debugger suspension and pulses the GC mode to block before acquiring the lock.
The fix is only a heuristic and lessens the problem when it is detected that the lock is held by some thread. Since the lock is acquired in preemptive GC mode, it is still possible that after the check at the start of a FuncEval, another thread acquires the lock and the FuncEval may time out. The polling makes it less likely for the lock to be taken by background tiering work, for example if a FuncEval starts while rejitting a method.
The expression evaluation experience may be worse when it is detected that the lock is held, and may still happen from unfortunate timing

Fix for #1537

- In many cases cooperative GC mode is entered after acquiring the slot backpatching lock and the thread may block for debugger suspension while holding the lock. A FuncEval may time out on entering the lock if for example it calls a virtual or interface method for the first time. Failing the FuncEval when the lock is held enables the debugger to fall back to other options for expression evaluation. - Also added polls for debugger suspension before acquiring the slot backpatching lock on background threads that often operate in preemptive GC mode. A common case is when the debugger breaks while the tiering delay timer is active, the timer ticks shortly afterwards (after debugger suspension completes) and if a thread pool thread is already available, the background thread would block while holding the lock. The poll checks for debugger suspension and pulses the GC mode to block before acquiring the lock. - The fix is only a heuristic and lessens the problem when it is detected that the lock is held by some thread. Since the lock is acquired in preemptive GC mode, it is still possible that after the check at the start of a FuncEval, another thread acquires the lock and the FuncEval may time out. The polling makes it less likely for the lock to be taken by background tiering work, for example if a FuncEval starts while rejitting a method. - The expression evaluation experience may be worse when it is detected that the lock is held, and may still happen from unfortunate timing Fix for dotnet#1537

- In many cases cooperative GC mode is entered after acquiring the slot backpatching lock and the thread may block for debugger suspension while holding the lock. A FuncEval may time out on entering the lock if for example it calls a virtual or interface method for the first time. Failing the FuncEval when the lock is held enables the debugger to fall back to other options for expression evaluation. - Also added polls for debugger suspension before acquiring the slot backpatching lock on background threads that often operate in preemptive GC mode. A common case is when the debugger breaks while the tiering delay timer is active, the timer ticks shortly afterwards (after debugger suspension completes) and if a thread pool thread is already available, the background thread would block while holding the lock. The poll checks for debugger suspension and pulses the GC mode to block before acquiring the lock. Risks: - The fix is only a heuristic and lessens the problem when it is detected that the lock is held by some thread. Since the lock is acquired in preemptive GC mode, it is still possible that after the check at the start of a FuncEval, another thread acquires the lock and the FuncEval may time out. The polling makes it less likely for the lock to be taken by background tiering work, for example if a FuncEval starts while rejitting a method. - The expression evaluation experience may be worse when it is detected that the lock is held, and may still happen from unfortunate timing - Low risk for the change itself Port of dotnet/runtime#2380 Fix for dotnet/runtime#1537

noahfalk

LGTM, thanks Kount!

* Revert "Revert "Disable test based on #129 (#130)" (#2310)" This reverts commit 427cd91. * Revert "Fail FuncEval if slot backpatching lock is held by any thread (#2380)" This reverts commit fc06054. * Revert "Improve call counting mechanism (#1457)" This reverts commit 3a457cb.

…28006) - In many cases cooperative GC mode is entered after acquiring the slot backpatching lock and the thread may block for debugger suspension while holding the lock. A FuncEval may time out on entering the lock if for example it calls a virtual or interface method for the first time. Failing the FuncEval when the lock is held enables the debugger to fall back to other options for expression evaluation. - Also added polls for debugger suspension before acquiring the slot backpatching lock on background threads that often operate in preemptive GC mode. A common case is when the debugger breaks while the tiering delay timer is active, the timer ticks shortly afterwards (after debugger suspension completes) and if a thread pool thread is already available, the background thread would block while holding the lock. The poll checks for debugger suspension and pulses the GC mode to block before acquiring the lock. Risks: - The fix is only a heuristic and lessens the problem when it is detected that the lock is held by some thread. Since the lock is acquired in preemptive GC mode, it is still possible that after the check at the start of a FuncEval, another thread acquires the lock and the FuncEval may time out. The polling makes it less likely for the lock to be taken by background tiering work, for example if a FuncEval starts while rejitting a method. - The expression evaluation experience may be worse when it is detected that the lock is held, and may still happen from unfortunate timing - Low risk for the change itself Port of dotnet/runtime#2380 Fix for dotnet/runtime#1537

kouvel added the area-TieredCompilation-coreclr label Jan 30, 2020

kouvel added this to the 5.0 milestone Jan 30, 2020

kouvel requested review from noahfalk and davidwrighton January 30, 2020 09:09

kouvel self-assigned this Jan 30, 2020

kouvel mentioned this pull request Jan 30, 2020

[3.1] Fail FuncEval if slot backpatching lock is held by any thread dotnet/coreclr#28006

Merged

noahfalk approved these changes Jan 30, 2020

View reviewed changes

kouvel merged commit fc06054 into dotnet:master Jan 30, 2020

kouvel deleted the LockFuncEval branch January 30, 2020 18:15

jkotas mentioned this pull request Feb 1, 2020

Improve call counting mechanism #1457

Merged

kouvel mentioned this pull request Mar 4, 2020

Func-eval while runtime is suspended may not run due to deadlock (timing issue) #1537

Closed

ghost locked as resolved and limited conversation to collaborators Dec 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fail FuncEval if slot backpatching lock is held by any thread #2380

Fail FuncEval if slot backpatching lock is held by any thread #2380

kouvel commented Jan 30, 2020

noahfalk left a comment

Fail FuncEval if slot backpatching lock is held by any thread #2380

Fail FuncEval if slot backpatching lock is held by any thread #2380

Conversation

kouvel commented Jan 30, 2020

noahfalk left a comment

Choose a reason for hiding this comment