Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix race condition that caused LaunchAsync to never resolve for chrome (
#2214) * Fix #2202 Only process received messages after the discovered targets have been stored. * Change the await to only apply to attachedToTarget, leaving other messages unchanged. This fixes some of the unit tests, which were failing due to changes in the order of execution of initialization messages. * Remove the await for OnAttachedToTarget call, and also included a missing return when ignoring a target. * * Fixed a race condition if a message is received before the Browser._logger field is initialized. * Fixed a deadlock that could happen if the connection is closed on the thread that is processing received messages. TaskQueue could not be disposed on the same thread that held the semaphore. * Fixed a race condition if targets are created/changed concurrently before the TargetHandler is registered as an event handler. * Previous commit introduced a new race condition. It was possible that thread A could invoke `TaskQueue.Dispose()` and set `_isDisposed = 1`, which would then allow thread B to finish work setting `_held = false` but without releasing the semaphore, and then thread A would attempt `_semaphore.Wait()` entering a deadlock. * It was possible for the TargetManager initialization to finish without having discovered all targets. This was causing unit tests such as PuppeteerConnectTests.ShouldSupportTargetFilter to fail because the test executed faster than the target discovery. * PR review * Rolling back Target.setDiscoverTargets to be sent from the constructor * Handle exceptions in OnAttachedToTarget * OnAttachedToTarget should be executed synchronously if possible, so that new targets are added to `_attachedTargetsByTargetId` inside of the semaphore. Also fixes `Page.CloseAsync()` which was returning before `Target.CloseTask` resolved. This affected BrowserContextTests.ShouldFireTargetEvents on which it was possible for the test to finish before the `TargetDestroy` event. * Fix PuppeteerConnectTests.ShouldSupportTargetFilter. It was possible for the InitializeAsync to finish without all targets being initialized, and consequently the test would read an empty list of targets. The _targetDiscoveryCompletionSource should be awaited before logic that depends on _targetsIdsForInit inside of message processing, to make sure this collection was already initialized during the browser launch. * Fix OOPIFTests.ShouldDetectExistingOopifsWhenPuppeteerConnectsToAnExistingPage. Disposing the `browser1` was closing the page, which then caused the `Page.CloseAsync()` in `PuppeteerPageBaseTest` to fail. The test code now matches upstream puppeteer. * Revert unintentional line ending changes. * Use the launcher timeout when awaiting for `_targetDiscoveryCompletionSource`, as a defensive measure against deadlocks.
- Loading branch information
1 parent
f5b5d87
commit e0df7f7
Showing
7 changed files
with
153 additions
and
45 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.