Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panel.onDidDispose can't reliable be used to avoid exceptions #48509

Closed
dbaeumer opened this issue Apr 24, 2018 · 3 comments
Closed

panel.onDidDispose can't reliable be used to avoid exceptions #48509

dbaeumer opened this issue Apr 24, 2018 · 3 comments
Assignees

Comments

@dbaeumer
Copy link
Member

Tests: #48453

The documentation suggests that panel.onDidDispose can reliable be used to avoid exception. I think this is not true since the communication between the renderer and extension host is async. It can happen that the user presses the close button and the extension tries to access the WebView before the dispose event made it to the extension host. We should either make this clear in the doc or handle these cases more gracefully.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 25, 2018

Good point, although I believe this is mitigated by how dispose is implemented. The important points:

  • The extension host throws if you try accessing a disposed webview. The main thread does not.
  • The main thread throws if you try accessing an unknown webview.

Here's what happens when the user closes a panel:

  1. The onDispose callback is invoked in mainThreadWebview
  2. The callback sends a onDidDisposeWebview message to the extension host.
  3. The extension host receives this message. It marks its ExtHostWebview object as being disposed and fires the extension onDidDispose method which synchronously invokes all the listeners. If an extension tries to call method on the webview object at this point, an error will be thrown
  4. Meanwhile, the mainThreadWebview has been waiting for the extension host's onDidDiposeWebview event to complete. If the extension host tries to access the disposed webview while the mainThreadWebview is waiting, no error should occur because the main thread still knows about this webview
  5. Once onDidDisposeWebview completes in the mainThreadWebview, we delete the webview. At this point, any future attempts to access it will throw errors

Do you think that this mitigates the concern?

@dbaeumer
Copy link
Member Author

What happens if the extension does something with the web view between 1. and 3. If this returns reasonable results and doesn't throw I am fine with closing the issue.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 26, 2018

Getters return the current values, while all the setters and methods no-op

@mjbvz mjbvz closed this as completed Apr 26, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 10, 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

No branches or pull requests

2 participants