microsoft / AzureStorageExplorer

Easily manage the contents of your storage account with Azure Storage Explorer. Upload, download, and manage blobs, files, queues, tables, and Cosmos DB entities. Gain easy access to manage your virtual machine disks. Work with either Azure Resource Manager or classic storage accounts, plus manage and configure cross-origin resource sharing (CORS) rules.
Creative Commons Attribution 4.0 International
370 stars 84 forks source link

Fix leaks around tab closing #8076

Closed craxal closed 1 week ago

craxal commented 1 month ago

PR 554808 exposed a bug related to closing tabs. Specifically, a call to raise the blob-container.close event never returns. Thus, any code following an awaited call won't be executed.

The underlying problem is due to the WebviewProxy disposing of a webview while it is trying to fulfill a request. A disposed webview cannot send a response, which results in a function call that never returns, as illustrated in this (simplified) diagram:

sequenceDiagram
autonumber

  box Blob extension process
    participant Blob Extension
    participant Blob Host
  end
  box Gray Explorer Process
    participant Explorer
    participant Explorer Host
  end
  box Gray Renderer Process
    participant Renderer Host
    participant Webview
    participant Panel API
  end

  Blob Extension ->> Blob Host: raise("blob-close")
  Blob Host -->> Renderer Host: request("raise", "blob-close")
  Renderer Host ->> Webview: handle("blob-close")
  Webview -->> Explorer Host: request("raise", "blob-close")
  Explorer Host ->> Explorer: handle("blob-close")
  Explorer ->> Explorer Host: execute("panel-close")
  Explorer Host -->>+ Renderer Host: request("execute", "panel-close")
  Renderer Host ->>+ Panel API: closePanel()
  Panel API ->>+ Webview: dispose()
  Webview -X Explorer: DISPOSE
  Webview -X Explorer Host: DISPOSE
  Webview ->>- Panel API: _
  Panel API ->>- Renderer Host: _
  Renderer Host --X- Explorer Host: response("execute", "panel-close")

See #7976, #7989.

craxal commented 3 weeks ago

A solution will require more careful management of a webview's lifecycle. Several details that need to be considered are:

craxal commented 3 weeks ago

After more investigation, I don't think the previously mentioned solution is good enough. The problem seems to be inherent with explorers trying to close themselves. The crux of the problem is that the close() panel API immediately removes the <webview> element from the DOM, which destroys its corresponding view model and renderer process (including the explorer and host). But because the request to close came from the explorer itself, it doesn't get a chance to respond.

To solve this problem we can do the following: