Skip to content
This repository was archived by the owner on Jan 28, 2023. It is now read-only.

Revert "windows: Use queued spin locks"#48

Merged
wcwang merged 1 commit intomasterfrom
ept-deadlock-fix
May 2, 2018
Merged

Revert "windows: Use queued spin locks"#48
wcwang merged 1 commit intomasterfrom
ept-deadlock-fix

Conversation

@raphaelning
Copy link
Contributor

This reverts commit 5cfb841.

The said patch is seriously flawed:

  1. It calls the wrong Windows Driver Kit functions. E.g.,
    KeAcquireInStackQueuedSpinLockAtDpcLevel() should only be called
    when IRQL = DISPATCH_LEVEL, but most of HAXM code runs below
    that level. The more general KeAcquireInStackQueuedSpinLock()
    should be used instead.
  2. Unlike the release of a regular spin lock, which is a simple
    operation, the release of a queued spin lock needs to wait for a
    condition and thus may block. If called from a function that
    should never block, e.g. ept_tree_alloc_page(), a deadlock may
    occur.

Fall back to regular spin locks which do not block on release.

This reverts commit 5cfb841.

The said patch is seriously flawed:

1. It calls the wrong Windows Driver Kit functions. E.g.,
   KeAcquireInStackQueuedSpinLockAtDpcLevel() should only be called
   when IRQL = DISPATCH_LEVEL, but most of HAXM code runs below
   that level. The more general KeAcquireInStackQueuedSpinLock()
   should be used instead.
2. Unlike the release of a regular spin lock, which is a simple
   operation, the release of a queued spin lock needs to wait for a
   condition and thus may block. If called from a function that
   should never block, e.g. ept_tree_alloc_page(), a deadlock may
   occur.

Fall back to regular spin locks which do not block on release.
@raphaelning raphaelning requested a review from wcwang May 2, 2018 06:58
@wcwang wcwang merged commit 6496c38 into master May 2, 2018
@wcwang wcwang deleted the ept-deadlock-fix branch May 2, 2018 08:35
@raphaelning raphaelning mentioned this pull request Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants