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

Error?: Webview not freeing up callback resources after webview.run() #130

Open
JorchRL opened this issue Jun 30, 2022 · 5 comments
Open
Labels
bug Something isn't working

Comments

@JorchRL
Copy link

JorchRL commented Jun 30, 2022

As far as I understand, the webview.destroy() method should free up all resources as it should run unbind() once the webview window is closed. But I am getting an error from Deno's test sanitizer. The resources from bound callbacks are not getting closed.

console.log(Deno.resources()) just after webview.run() yields the following:

------- output -------
{
  "0": "stdin",
  "1": "stdout",
  "2": "stderr",
  "3": "dynamicLibrary",
  "4": "unsafecallback"
}

I'll include a small test case to reproduce the problem. Let me know if you are aware of the issue, or if I am making some mistake.

Also, if it is something that needs fixing, how may I be able to help! 😄

How to reproduce.

Run the following test

import { Webview } from "https://deno.land/x/[email protected]/mod.ts";

Deno.test("Webview resources", () => {
  const app = new Webview();
  app.bind("test", () => {
    console.log("test");
  });
  app.run();
});

Once the window appears, close it manually. And the test will fail
Screenshot from 2022-06-29 20-04-12

my system

Ubuntu: 22.04 LTS
Deno: 1.23.1
Webview_deno: 0.7.3

@eliassjogreen eliassjogreen added the bug Something isn't working label Jun 30, 2022
@eliassjogreen
Copy link
Member

Strange, what happens if you manually call destroy after the run call, or even the unload function? Could be an issue with Deno.test or more likely an issue with webview_deno.

@JorchRL
Copy link
Author

JorchRL commented Jun 30, 2022

Let me try that out. The issue is made explicit due to Deno's test sanitizer, if I understand correctly. So in a normal script the program ends with no visible issue.

Also, I tried running unbind() manually and it does free the callback resources. Let me try some stuff and I'll report what I find 😄

@JorchRL
Copy link
Author

JorchRL commented Jul 4, 2022

Here's an update. Running destroy after run does not fix the error. But unbinding the function before run does free the resources.

Unbinding the function results in the unsafeCallback being closed.

And running unload() after run() frees up the "dynamicLibrary" resource (which causes another warning but I don't think its an issue here)


What I can guess is the following:

  • closing the window and/or running destroy() should free the callback resources. So maybe there is a problem with destroy().

However, while debugging the test. I see that when destroy() is called, the webview variable holds 1 value on its #callbacks field with the "test" callback I bound. Which is expected.
image

Taking a step into it:

image

And on the next step it seems that const callback of Object.keys(this.#callbacks) evaluates to undefined.
image

And as expected, the execution skips the entire for loop.

image

Therefore, I'm pretty confident that's the problem. That would explain why neither destroy() ending run() frees up the resources.

Hopefully that pinpoints the error!

@JorchRL
Copy link
Author

JorchRL commented Jul 4, 2022

Maybe the problem is that #callbacks is a Map. And Object.keys() is not behaving as expected?

@JorchRL
Copy link
Author

JorchRL commented Jul 4, 2022

Here is what I tried:

First change destroy() to

destroy() {
    for (const callback of this.#callbacks) {
      this.unbind(callback[0]); // just get the string name of the callback. 
    }
    lib.symbols.webview_terminate(this.#handle);
    lib.symbols.webview_destroy(this.#handle);
    this.#handle = null;
  }

And then, after debugging again. When unbind is called from destroy(), the test ends at lib.symbols.webview_unbind() (see snippet below) with ...segmentation fault (core dumped) It doesn't fail. It just ends. I honestly don't know what to make of it 😅

 unbind(name: string) {
    lib.symbols.webview_unbind(this.#handle, encodeCString(name)); // <-- here it fails 
    this.#callbacks.get(name)?.close();
    this.#callbacks.delete(name);
  }

I'm sorry if this is not super helpful. I have no idea how to further debug this problem


As unbind() by itself seems to work fine. They workaround I have is to manually keep track of the bound functions and closing their resources manually once run() finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants