Deadlock while closing the window with persistent threads running

[pomplesiegel]

When updating the DOM from a separate thread, using calls like webview.evaluate_js() I found it quite useful to have an API call which indicates whether the window is still present / attempting to close.

Problem

If one clicks the close button on a window (rather than quitting it like with Apple + Q), and then a separate thread calls to the webview object, like webview.evaluate_js(), this thread becomes blocked and stuck in that call indefinitely.

What would be helpful is a function like
webview.window_exists()

which returns a boolean, which is set within WindowDelegate::windowWillClose_() (and the equivalent location on other platforms). I tested this by adding a global variable to cocoa.py which is set within windowWillClose_, and having webview return the status of this boolean. Certainly not the cleanest way, but it works.

Could we perhaps integrate this in a more OO-friendly fashion. I'm happy to help.

Alternatively, we could provide an API for a callback function, which would be called (in addition to the window closing) when the user hits the "x" or close button on a window. A sort-of cleanup call, to prevent deadlock.

Thoughts?
Thanks!
Michael

[shivaprsd]

Hm, looks like this is an issue we should solve internally, rather than exposing
it to the API. pywebview already handles calls that occur before the window is
created; the same could be done for calls that occur after. Another way is to
really quit the app once the window is closed, just as if ⌘Q was pressed.

I think the cleanup-call feature is more plausible, especially in the later context.
This would be an addition to the already implemented confirm_quit parameter.
Thanks for the suggestion, I'll look into it and get back to you. 🙂

BTW to check if the app is still running on Cocoa, you can use the isRunning()
method of BrowserView.app, not need to keep a global variable.

[pomplesiegel]

That would be great. Any thoughts of the equivalent call for WinForms?

[shivaprsd]

Roman does all Windows coding so I don't know much about it :) Nevertheless,
there doesn't seem to be an easy built-in way for this on WinForms; a global var
should be the best option.

[r0x0r]

The deadlock must certainly be fixed, but I don't feel there is a need for a public window_exists function (unless multi-window functionality materializes). A better approach would be to catch windowWillClose/OnDestroy/etc event on each platform and a set a semaphore there, evaluate_js would acquire the semaphore and would check for the existence of the window before executing javascript code.

[shivaprsd]

I just noticed, in its current shape, the multi-window code automatically handles
this. The get_instance method there raises an exception if the user calls an API
on a non-existent window, which breaks the deadlock. Here's a screenshot:

Setting a new semaphore would need adding it to all the API methods, right?

[pomplesiegel]

@shivaprsdv, Any thoughts on the best path forward for this one? I have a single-window application, but should I use the multi-window branch simply for this behavior? Currently there is no elegant way to quit a program with persistent threads running.

[shivaprsd]

I have confirmed that we have indeed hit a bug here. This occurs on all platforms
I have tested. A fix for Cocoa is up, could get some help with testing. It internally
uses the same isRunning() function that I mentioned.

@pomplesiegel Feel free to experiment with the multi-window branch anyway.
Even if your app is single-window, it should not incur any overheads. I wrote that
code, with inputs from Roman and others; it is still in testing, so any feedback is
well appreciated. 🙂

[pomplesiegel]

Great, i'll give the multi-window a shot. Thanks!

[pomplesiegel]

@shivaprsdv, I just tested your branch "deadlock-fix" and it fixed my thread problem for quitting the program! Great work.

[shivaprsd]

Great, thanks. 🙂

But I should say it's likely to be only a temporary fix. For the problem seems to
be of a much general nature, and warrants a more solid, cross-platform solution.
We'll keep this issue open and updated.