Skip to content

fix(transport): throw TargetClosedException for requests on disposed connections#3328

Open
mete0rfish wants to merge 2 commits into
microsoft:mainfrom
mete0rfish:fix-3326
Open

fix(transport): throw TargetClosedException for requests on disposed connections#3328
mete0rfish wants to merge 2 commits into
microsoft:mainfrom
mete0rfish:fix-3326

Conversation

@mete0rfish

Copy link
Copy Markdown

Fixed #3326

Summary

  • Return TargetClosedException for operations attempted after IPlaywright.Dispose().
  • Prevent the internal SemaphoreSlim disposal error. (System.ObjectDisposedException)
  • Complete pending callbacks during connection disposal.

Before

sequenceDiagram
      participant User as User Code
      participant PW as PlaywrightImpl
      participant Conn as Connection
      participant Queue as TaskQueue
      participant Handle as JSHandle

      User->>PW: playwright.Dispose()
      PW->>Conn: _connection.Dispose()
      Conn->>Queue: _queue.Dispose()
      Queue->>Queue: SemaphoreSlim.Dispose()
      Conn-->>PW: disposed

      User->>Handle: await handle.DisposeAsync()
      Handle->>Conn: SendMessageToServerAsync("dispose")
      Conn->>Conn: _closedError is null
      Conn->>Queue: EnqueueAsync()
      Queue->>Queue: SemaphoreSlim.WaitAsync()
      Queue--xConn: ObjectDisposedException
      Conn--xHandle: ObjectDisposedException
      Handle--xUser: ObjectDisposedException
Loading
  • playwright.Dispose() disposes the TaskQueue without _closedError.
  • A later handle cleanup attempts to access the disposed SemaphoreSlim.
  • The internal ObjectDisposedException is exposed to the user.

After

sequenceDiagram
      participant User as User Code
      participant PW as PlaywrightImpl
      participant Conn as Connection
      participant Queue as TaskQueue
      participant Handle as JSHandle

      User->>PW: playwright.Dispose()
      PW->>Conn: _connection.Dispose()
      Conn->>Conn: Check and set _disposed
      Conn->>Conn: Set _closedError to TargetClosedException
      Conn->>Conn: Fail and clear pending callbacks
      Conn->>Queue: _queue.Dispose()
      Queue->>Queue: SemaphoreSlim.Dispose()
      Conn-->>PW: disposed

      User->>Handle: await handle.DisposeAsync()
      Handle->>Conn: SendMessageToServerAsync("dispose")
      Conn->>Conn: Detect _closedError
      Conn--xHandle: TargetClosedException
      Handle->>Handle: Catch target-closed error
      Handle-->>User: completed without exception
Loading
  • The connection is marked as closed before pending callbacks and resources are disposed.
  • Later requests fail with TargetClosedException without accessing the TaskQueue.
  • Handle cleanup recognizes the closed connection and completes without an exception.

…connections

Disposed connections reject new requests by throwing TargetClosedException, rather than exposing ObjectDisposedException from the task queue.
@mete0rfish

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree [company="{your company}"]

@mete0rfish please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@mete0rfish

Copy link
Copy Markdown
Author

@mete0rfish the command you issued was incorrect. Please try again.

Examples are:

@microsoft-github-policy-service agree

and

@microsoft-github-policy-service agree company="your company"

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: JSHandle.DisposeAsync throws raw ObjectDisposedException on SemaphoreSlim when Connection is disposed

1 participant