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

Browser reference held at Shutdown causing crash #1

Closed
captainjono opened this issue Oct 10, 2020 · 17 comments
Closed

Browser reference held at Shutdown causing crash #1

captainjono opened this issue Oct 10, 2020 · 17 comments

Comments

@captainjono
Copy link
Owner

With the sample application, when you close it will display a failure message because the call to CefRuntime.ShutDown() fails to execute because CefBrowser is being held in memory still.

My analysis is currently pointing towards the shutdown sequence not being correctly implemented and this only manifests on certain platforms in this way. As i starte debugging the shutdown sequence as per

https://magpcss.org/ceforum/viewtopic.php?f=6&t=17314
https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=17696#p46124
https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=17669

i came across this is the source code

          if (Browser != null)
                {
                    // A hack - until fix is found for why OnBeforeClose is not called in CefGlueLifeHandler.cs
                    Browser?.OnBeforeClose();

                    Browser?.Dispose();
                    Browser = null;
                    _browserWindowHandle = IntPtr.Zero;
                }

which seems to indicate that CEF is not alerting the app at all the browser window has closed. My main suspect so far is the messageloop not draining correctly, I am in the process of trying different ways to wait, post delay task. etc to see if i can find a combo that works.

im experimenting in this branch
https://github.com/captainjono/Chromely/tree/xammac-fix-shutdown

not there yet... lots of leads

cefsharp/CefSharp#3047
chromelyapps#237

https://magpcss.org/ceforum/viewtopic.php?f=6&t=17314

thread 0 Crashed::CrBrowserMain Dispatch queue: com.apple.main - thread
0   libcef                          0x0000000115acbf2c logging::LogMessage::~LogMessage() + 2828(logging.cc:953)
1   libcef                          0x0000000111745a8a shutdown_checker::AssertNotShutdown() + 106(shutdown_checker.cc:54)
2   libcef                          0x0000000111799e8f CefLifeSpanHandlerCToCpp::OnBeforeClose(scoped_refptr<CefBrowser>) + 31(life_span_handler_ctocpp.cc:150)
3   libcef                          0x0000000115777c9b CefBrowserHostImpl::DestroyBrowser() + 203(browser_host_impl.cc:1581)
4   libcef                          0x000000011578c57b CefBrowserInfoManager::DestroyAllBrowsers() + 347(browser_info_manager.cc:352)
5   libcef                          0x000000011579b552 CefContext::FinishShutdownOnUIThread(base::WaitableEvent *) + 114(context.cc:680)
6   libcef                          0x000000011579acdf CefContext::Shutdown() + 303(context.cc:569)
7   libcef                          0x000000011579ab27 CefShutdown() + 151(context.cc:351)
8 ??? 0x0000000110c5754d 0 + 4576343373
9 ??? 0x0000000110b97f6b 0 + 4575559531
10 ??? 0x0000000110b97d83 0 + 4575559043
11 ??? 0x000000010d0a345b 0 + 4513739867
12  libchromely.dylib               0x000000010f820f22 createwindow + 290
13 ??? 0x000000010f812f73 0 + 4555091827
14 ??? 0x000000010f811903 0 + 4555086083
15 ??? 0x000000010f8108f3 0 + 4555081971
16 ??? 0x000000010f7a6ba0 0 + 4554648480
17 ??? 0x000000010f7a56e3 0 + 4554643171
18 ??? 0x000000010f7a5429 0 + 4554642473
19 ??? 0x000000010d00deeb 0 + 4513128171
20 ??? 0x000000010d00cafb 0 + 4513123067
21 ??? 0x000000010cf87df2 0 + 4512579058
22 ??? 0x000000010af15ad3 0 + 4478556883
23 ??? 0x000000010af15ec1 0 + 4478557889
24  com.captainjonoo.Chromely - XamMac    0x00000001088512ce mono_jit_runtime_invoke + 1550
25  com.captainjonoo.Chromely - XamMac    0x0000000108986368 mono_runtime_invoke_checked + 136(object.c:3220)
26  com.captainjonoo.Chromely - XamMac    0x000000010898d695 mono_runtime_exec_main_checked + 117(object.c:5284)
27  com.captainjonoo.Chromely - XamMac    0x00000001087ad73c mono_jit_exec + 364(driver.c:1328)
28  com.captainjonoo.Chromely - XamMac    0x00000001087b0836 mono_main + 8790(driver.c:2715)
29  com.captainjonoo.Chromely - XamMac    0x00000001087657bc xamarin_main + 1116(launcher.m:674)
30  com.captainjonoo.Chromely - XamMac    0x0000000108766604 main + 36(launcher.m:693)
31  libdyld.dylib                   0x00007fff6c919cc9 start + 1
@captainjono
Copy link
Owner Author

captainjono commented Oct 11, 2020

Adding some research here:

My debugging has lead me to CefLifeSpanHandler.DoClose() and learning more about the CEF destruction cycle.
There is some interaction going on between the CloseBrowser(), DoClose() and the terminating of the app and the closing of the window. I see that returning true from it mkeans we are handling the close ourselves.

https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=17668#p45915

but whenever i close the window, it calls CefShutdown.... hmmm.. We dont need to worry too much about the lifecycle apparently if we dont need to support the JS exit.
https://www.magpcss.org/ceforum/viewtopic.php?f=17&t=14415

apparently calling OnBeforeClose() is a valid thing to be doing. is it just a matter of switching these calls around? onbeforeclosed should be called after Dispose()? after DoWork returns true?

@captainjono
Copy link
Owner Author

My take on this this

...
2   libcef                          0x0000000111799e8f CefLifeSpanHandlerCToCpp::OnBeforeClose(scoped_refptr<CefBrowser>) + 31(life_span_handler_ctocpp.cc:150)

is saying that - at shutdown - ceflifespanhandler still has a reference to the browser..

so in the DoClose of CefGlueLifeSpanHandler i added a call to this.dispose(true)
and now the stack is



            Disposing of Chromely Window
Telling browser to close
CefGlueBrowser entering dispose
CefGlueBrowser closebrowser true
Disposed of Chromely Window
Quitmessageloop delayed
[Info] CEF ShutdownCallback
[Info] CEF TERMINATE -quit callback
Thread finished: < Thread Pool > #5
Thread finished: < Thread Pool > #9
Thread finished: < Thread Pool > #4
Thread finished: < Thread Pool > #10
Thread finished: < Thread Pool > #8
Thread finished: < Thread Pool > #6
Thread finished: < Thread Pool > #7
Thread started: < Thread Pool > #11
Thread started: < Thread Pool > #12
Thread finished: < Thread Pool > #3
Window Onclose called
Thread started: < Thread Pool > #13
[Error] Foundation.ObjCException: NSInternalInconsistencyException: < NSView 0x7ffde5640db0 > has reached dealloc but still has a super view. Super views strongly reference their children, so this is being over - released, or has been over-released in the past.
  at ObjCRuntime.Runtime.ThrowNSException(System.IntPtr ns_exception)[0x00001] in / Library / Frameworks / Xamarin.Mac.framework / Versions / 6.20.2.2 / src / Xamarin.Mac / ObjCRuntime / Runtime.cs:405
  at ObjCRuntime.Runtime.throw_ns_exception(System.IntPtr exc)[0x00001] in / Users / builder / jenkins / workspace / xamarin - macios / xamarin - macios / runtime / Delegates.generated.cs:128
  at(wrapper native - to - managed) ObjCRuntime.Runtime.throw_ns_exception(intptr)
  at(wrapper managed - to - native) Chromely.Native.MacNativeMethods.createwindow(Chromely.Native.MacNativeMethods / ChromelyParam &)
  at Chromely.Native.MacCocoaHost.CreateWindow(Chromely.Core.Configuration.IWindowOptions options, System.Boolean debugging)[0x001b2] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Native / MacCocoa / MacCocoaHost.cs:75
  at Chromely.Windows.NativeWindow.ShowWindow()[0x00001] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / NativeWindow.cs:35
  at Chromely.Windows.Window..ctor(Chromely.Core.Host.IChromelyNativeHost nativeHost, Chromely.Core.IChromelyContainer container, Chromely.Core.Configuration.IChromelyConfiguration config, Chromely.Core.Network.IChromelyCommandTaskRunner commandTaskRunner, Xilium.CefGlue.Wrapper.CefMessageRouterBrowserSide browserMessageRouter)[0x00083] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / Window.cs:48
  at Chromely.Windows.ChromelyWindow.CreateMainView()[0x0001b] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / ChromelyWindow.cs:72
  at Chromely.CefGlue.BrowserWindow.HostBase.CreateMainWindow()[0x0003b] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:278
  at Chromely.CefGlue.BrowserWindow.HostBase.RunInternal(System.String[] args)[0x00224] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:255
  at Chromely.CefGlue.BrowserWindow.HostBase.Run(System.String[] args)[0x00002] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:92
2020 - 10 - 12 11:36:51.916 Chromely.XamMac[15773:2042246] * **Assertion failure in -[NSView dealloc], / AppleInternal / BuildRoot / Library / Caches / com.apple.xbs / Sources / AppKit / AppKit - 1894.60.100 / AppKit.subproj / NSView.m:1682

this feels like progress. It feels like shutdown is working now after i explicitly dispose of the ceflifetimehandler.

this would indicate the destructor is possible not being called.

        ~CefLifeSpanHandler()
        {
            Dispose(false);
        }

but it also indicates that the destructor may not be called because something has a reference to this view??

@captainjono
Copy link
Owner Author

captainjono commented Oct 12, 2020

https://www.magpcss.org/ceforum/viewtopic.php?f=10&t=17423

Found this indicating that to close the browser we need to remove the view. ^^

// |-tryToTerminateApplication:| differs from the standard
// |-applicationShouldTerminate:| in that no special event loop is run in the
// case that immediate termination is not possible (e.g., if dialog boxes
// allowing the user to cancel have to be shown). Instead, this method tries to
// close all browsers by calling CloseBrowser(false) via
// ClientHandler::CloseAllBrowsers. Calling CloseBrowser will result in a call
// to ClientHandler::DoClose and execution of |-performClose:| on the NSWindow.
// DoClose sets a flag that is used to differentiate between new close events
// (e.g., user clicked the window close button) and in-progress close events
// (e.g., user approved the close window dialog). The NSWindowDelegate
// |-windowShouldClose:| method checks this flag and either calls
// CloseBrowser(false) in the case of a new close event or destructs the
// NSWindow in the case of an in-progress close event.

----------------------------

// ClientHandler::OnBeforeClose will be called after the CEF NSView hosted in
// the NSWindow is dealloc'ed.**

-------------------------

@captainjono
Copy link
Owner Author

Hmm im wondering if this is the reason onbeforeclose is not called? after adding in the explicit call to .Dispose() i have been able successfully call shutdown.. i think.. but something has not cleaned up the view correctly.

inside chromely_mac.mm

- (void)cleanup {
  // Don't want any more delegate callbacks after we destroy ourselves.
  window_.delegate = nil;
  window_.contentView = [[NSView alloc] initWithFrame:NSZeroRect];
  // Delete the window.
#if !__has_feature(objc_arc)
  [window_ autorelease];
#endif  // !__has_feature(objc_arc)
  window_ = nil;
 /// root_window_->OnNativeWindowClosed(); -     /// CHROMELYPARAM- function pointer?


  chromelyParam_.exitCallback();
}

@end  // @implementation RootWindowDelegate

i suspect the view isnt being removed properly from the heirachy?
im not sure window_.contentView = [[NSView alloc] initWithFrame:NSZeroRect]; cuts the mustard

https://stackoverflow.com/questions/23428616/how-to-remove-view-from-superview

            Disposing of Chromely Window
Telling browser to close
CefGlueBrowser entering dispose
CefGlueBrowser closebrowser true
Disposed of Chromely Window
Thread started: < Thread Pool > #8
[Info] CEF ShutdownCallback
[Info] CEF TERMINATE -quit callback
Window Onclose called
[Error] Foundation.ObjCException: NSInternalInconsistencyException: < NSView 0x7fa4675229c0 > has reached dealloc but still has a super view. Super views strongly reference their children, so this is being over - released, or has been over-released in the past.
  at ObjCRuntime.Runtime.ThrowNSException(System.IntPtr ns_exception)[0x00001] in / Library / Frameworks / Xamarin.Mac.framework / Versions / 6.20.2.2 / src / Xamarin.Mac / ObjCRuntime / Runtime.cs:405
  at ObjCRuntime.Runtime.throw_ns_exception(System.IntPtr exc)[0x00001] in / Users / builder / jenkins / workspace / xamarin - macios / xamarin - macios / runtime / Delegates.generated.cs:128
  at(wrapper native - to - managed) ObjCRuntime.Runtime.throw_ns_exception(intptr)
  at(wrapper managed - to - native) Chromely.Native.MacNativeMethods.createwindow(Chromely.Native.MacNativeMethods / ChromelyParam &)
  at Chromely.Native.MacCocoaHost.CreateWindow(Chromely.Core.Configuration.IWindowOptions options, System.Boolean debugging)[0x001b2] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Native / MacCocoa / MacCocoaHost.cs:75
  at Chromely.Windows.NativeWindow.ShowWindow()[0x00001] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / NativeWindow.cs:35
  at Chromely.Windows.Window..ctor(Chromely.Core.Host.IChromelyNativeHost nativeHost, Chromely.Core.IChromelyContainer container, Chromely.Core.Configuration.IChromelyConfiguration config, Chromely.Core.Network.IChromelyCommandTaskRunner commandTaskRunner, Xilium.CefGlue.Wrapper.CefMessageRouterBrowserSide browserMessageRouter)[0x00083] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / Window.cs:48
  at Chromely.Windows.ChromelyWindow.CreateMainView()[0x0001b] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely / Windows / ChromelyWindow.cs:72
  at Chromely.CefGlue.BrowserWindow.HostBase.CreateMainWindow()[0x0003b] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:278
  at Chromely.CefGlue.BrowserWindow.HostBase.RunInternal(System.String[] args)[0x00224] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:255
  at Chromely.CefGlue.BrowserWindow.HostBase.Run(System.String[] args)[0x00002] in / Users / janison / Desktop / Chromely / src_5.0 / Chromely.CefGlue / BrowserWindow / HostBase.cs:92
2020 - 10 - 12 15:00:00.429 Chromely.XamMac[16428:21

@captainjono
Copy link
Owner Author

captainjono commented Oct 12, 2020

So i made some mods to make tracing the shutdown sequence possible across the stack through the same log output so i could exactly the sequence things were coming in on and i have gotten to the point where i no longer get a crash on shutdown.

CreateWindow DemoApp
[Info] Running MacOSX chromium 83.0.4103.106
[Info] FROM libchromely -1
[Info] FROM libchromely 0
[Info] FROM libchromely 1
CefGlueLifeSpanHandler async AfterCreated
Created browser window. calling plugin
Disposing of Chromely Window
Telling browser to close
CefGlueBrowser entering dispose
CefGlueBrowser closebrowser true
calling CefGlueBrowser OnBeforeClosed explicitly
Disposed of Chromely Window
Quitmessageloop delayed
[Info] FROM libchromely 8
[Info] CEF ShutdownCallback
[Info] FROM libchromely 9
[Info] FROM libchromely 10
[Info] CEF TERMINATE - quit callback
Window Onclose called

i need todo more testing and then go over the changes and clean things up. but i feel like i am getting close!

Creating a PR to track the fix

@captainjono
Copy link
Owner Author

captainjono commented Oct 12, 2020

I note that the changes as i made them break using the 'X' to close the app. here is a log of the sequence, should be easy to work out a way to get both these sequences working!

image

mattkol added a commit to chromelyapps/Chromely that referenced this issue Oct 12, 2020
@mattkol
Copy link

mattkol commented Oct 12, 2020

@captainjono this is great.

So if I understand the findings so far, we need to:

  • Call Dispose(true) in LifeSpanHandler (Win, Linux, Mac?)
  • Remove the View on exit (Mac Only?)

On:

so in the DoClose of CefGlueLifeSpanHandler i added a call to this.dispose(true)
and now the stack is

For Windows I have tried:

    public class DefaultLifeSpanHandler : CefLifeSpanHandler
    {
        protected override bool DoClose(CefBrowser browser)
        {
            this.Dispose(true);
            return false;
        }
   }

Is that correct? Does not seem to be working for 5.0.
However, when I removed the hack for 5.1 it does work though. I just checked the fix - chromelyapps@fcb6e8e. I need to find sometime to check what the difference in, in behaviors. I usually do better on a weekend with a closer look.

I see that you are creating a PR, that is great.

Thanks.

@captainjono
Copy link
Owner Author

captainjono commented Oct 15, 2020

@mattkol

have u attempted to run this branch to verify what i am seeing? Although i am not gettting a crash anymore, i am seeing an interesting hang towards the end after quitmessageloop is called after window.onclose - the window goes unresponsive with high CPU and then exits after about 10sec.

image

If i run the app 10 times, most of the time it will do the above ^, but occasionally I will get a "successfully exited process" message and it doesnt hang. Ive tried adding in sleeps but nothing has helped.

regarding windows, I have not attempted to validate how these fixes behave across OS's yet as i dont have any issues at all on windows - process exit is smooth and error free. My app is going to production soon only for mac, so this exit crash is becoming critical for me to solve and holds all my attention at the moment :)

Im still integrating these fixes back into my codebase - i havnt successfully exited with my full app yet - im trying to chase down the reasons why https://www.magpcss.org/ceforum/viewtopic.php?f=14&t=17925

i think im and holding onto a reference somehow and am refactoring atm to isolate any cef refernces to a single point so i can dispose of them predictably. Do i need todo anything specific if i am using keyboard handlers? do i need to dispose of them at the time when i close the browser or is that supposed to be taken care of for me by chromely or it doesnt matter?

@captainjono
Copy link
Owner Author

captainjono commented Oct 19, 2020

@mattkol
ive been testing this solution more and i am getting different behaviour on different real mac devices indicating there is still some kind of threading issue still at play. The GC process also complicates things. The advice is to remove all references to CEF, but removing all references to chromely on shutdown will cause the GC to collect the xam-mac binding objects also.

With the extra changes i have made around the cleanup process, i am now getting this error this when integrating back into my app
image
all attempts to symbolise that stack have failed. from what i can gather this means a component has possibly been destroyed at the mono binding boundary & obj-c can no longer call back into it. This happens after i call quitmesageloop in cefgluelifespan.onclose

here are some other errors and what i think there meaning is
image
this is a nullref exception

I saw this sometimes as i was debugging, im hoping by posting these it can trigger some thoughts
image

image

image

My hunch is the current destruction sequence is causing all the components to fall apart instead of just closing the browser, releases the references then quitting the message loop. By the time quit is called, it feels like the subsystems are cleaning up resources and depending on what is on the stack depends on the luck at succeeding in the shutdown flow.

image

@mattkol
Copy link

mattkol commented Oct 20, 2020

@captainjono I am somehow losing track. In a previous comment I believe you were preparing a PR for the fix. So is this back to square one?

I will be able to help over the weekend. So just run this repo - https://github.com/captainjono/Chromely/tree/xammac/Chromely.XamMac?

@captainjono
Copy link
Owner Author

captainjono commented Oct 20, 2020

@mattkol basically i fixed some aspects of the shutdown process, but not all of them.

There is a threading issue somewhere that is occouring during the shutdown sequence and cleaning up objects somewhere that is causing libchromely and the main app to fall apart. Thats all i can really gather from the error codes so far.

Depending on the CPU core count and the code in memory at the time the issue may present itself or not.

to summarise:
I am definately trying to build a PR to fix the issue. Just not there yet. It passes testing on 1 device and fails on another device with the above error. I have been attempting to debug, but i cant get much info from the stack.
-I think i need to find a better way to debug and trace into the obj-c code from Chromely?
-I think its an interaction between GC / Xam.Mac / Obj-C cleanup / deallocate.
-this code added to the boostrapper will help to surface these errors

new Thread(() => 
{
  while(true) 
  {
     Thread.Sleep(30);
      GC.Collect(2);
   }
}

Stelzi79 pushed a commit to Stelzi79/Chromely that referenced this issue Oct 27, 2020
@captainjono
Copy link
Owner Author

captainjono commented Nov 2, 2020

Over the last week i have been able to perform extensive testing.

  • The issues i thought were threading related seem to be more closely manifesting as deallocation issues in various forms that were aggrevated by the multi-threaded-message-loop. The code involved is partially managed by Obj-C, partially by XamMac, and partially by CEF. In the window that displays the browser, the surface is referened and manipulated by all 3 systems.

  • I audited all the shutdown code and removed calls which were not required to satifsy Cef.Shutdown() semantics. Further to this, I negatively tested and can repo .CefRefPtr<> shutdown errors by navigating to a url and shutting down while the request is inflight. So i dont think the leftover errors are todo with CEF. That said,

    1. I note a shutdown crash that was patched a few days after 4103. This could cause an unwanted callback during shutdown...
  • During my investigation, i found that the only real reliable way i have found to ID a crash occouring on exit has been log messages (after a while your system will stop reporting crashes due to "Too many corpses"). The messages have been indicating intermittant failure in the xam-macos bindings... it seems mulitple bugs:

    1. FIXED GC'd callbacks: MacBrowserHost was declaring marsheled functions as privates, but they also needed to be static to stop GC'ing

    2. Unknown issue: causing intermittant crash on exit. Im not 100% sure whats going on here but this stack will happen 50% of the time when you run the DemoApp on the shutdown-crash branch. It will vary from system to system in terms of failure rate and is worse when memory is filling so its very likely todo with GC'ing.

    - source code of crashed line: https://bitbucket.org/chromiumembedded/cef/src/bf03589c4d7c450ecc37294e7ac47d0e8b12e403/libcef/browser/browser_host_impl.cc
    • i have also seen this variation

    The basic life-cycle of the window is:

  • Dotted and solid lines indicate stack level

  • Source code quick reference:

    • MacBrowserHost.cs: calls createwindow & hosts the Xam.mac callbacks
    • chromely_mac.mm: createwindow/deallocates/uses callbacks provided by xam.mac
    • Window.cs: creates the host given the hwind
    • CefGlueBrowser.cs: creates the host given the browser
    • ChromelyWindow.cs: the abstraction that wraps the native functions
  • The crash is manifesting by the call never returning back to XamMac from libChromely.dylib::createWindow

    • This is the use-case which caused the mono assert failure i have reported
      • I sprayed some statics around but they didnt help here...

On the failed shutdown, the logs stop after deallocation in chromely_mac.mm

This is a log of a graceful exit

  • The complexity of this process comes from CEF requiring us to close the browser, which essentially is triggering the window to close, which is triggering deallocation and GC'ing...

My next steps are to better understand who owns what:

  • understand if the mono assert is the bug causing the crash?

  • understand more about XamMac and autorelease pools and what i need todo here to satisfy the systems?

  • I am starting to seriously consider re-writing chromely_mac.mm / libChromely.dylib in XamMac so i can carefully define the lifetime of all the components. Thoughts?

@mattkol
Copy link

mattkol commented Nov 3, 2020

I am starting to seriously consider re-writing chromely_mac.mm / libChromely.dylib in XamMac so i can carefully define the lifetime of all the components. Thoughts?

I thought this was the original objective from the get-go - to have a replacement or parallel implementation of libchromely.dylib. If it is me, I will, because if it works well using XamMac (that has wider support) it is easier to retrofit libcrhomely.dylib or replace it entirely. On replacement as long as there is ownership/commitment going forward, I do not see why not.

@captainjono
Copy link
Owner Author

@mattkol

Thanks for the feedback. I wanted to re-write libchromely, but only if i had too (im currently under a very tight deadline and this is a large change to my system which has undergone extensive QA) :)

I am diving deep into the auto-release pools + ARC at the moment. I think a double dispose or similiar may be happening. Maybeits an simple as reparenting or removing before destroy. Im thinking about the xam-mac implementation at the same time. Im not seeing anything complex going on.

image

image

i found this code in on CEF forum that may be applicable

void MainWindow::OnBrowserClosed(CefRefPtr<CefBrowser> browser)
{
    CEF_REQUIRE_UI_THREAD();
...
    OnBrowserWindowDestroyed(browser);
    return;
}
void MainWindow::OnBrowserWindowDestroyed(CefRefPtr<CefBrowser> browser)
{
... specific cleanup
    browsers_destroyed_ = !_handler->HaveBrowsers(); // browser count is NOT > 0 ?
    if(browsers_destroyed_) { // yes, we have 0 browsers
        if (!window_destroyed_) { // main window not destroyed
            if(_handler->isClosing()) { // isClosing returns true if we entered the shutdown phase, for example click the Close button on main window
                CloseIfNeeded(true); // force close main window
            }
        }
    }
    NotifyDestroyedIfDone();
}
Note. browsers_destroyed_ and window_destroyed_ are bools, first indicates "all browsers are destroyed, browser count is 0" and the second means "main window is destroyed".

void MainWindow::CloseIfNeeded(bool force)
{
    if (GetSafeHwnd()) {
        if (force) {
            ::DestroyWindow(GetSafeHwnd());
        }
        else {
            ::PostMessage(GetSafeHwnd(), WM_CLOSE, 0, 0);
        }
    }
}
void MainWindow::OnDestroyed()
{
    window_destroyed_ = true;
    NotifyDestroyedIfDone();
}
Note. OnDestroyed is called from WM_NCDESTROY.

void MainWindow::NotifyDestroyedIfDone()
{
    // (re)check if browsers destroyed
    browsers_destroyed_ = !_handler->HaveBrowsers();
    if(window_destroyed_ && browsers_destroyed_) {
        OnWindowDestroyed();
    }
}
void MainWindow::OnWindowDestroyed()
{
    Application::QuitMessageLoop();
}

void Application::QuitMessageLoop()
{
  CefDoMessageLoopWork();
  CefQuitMessageLoop();
}

@mattkol
Copy link

mattkol commented Nov 3, 2020

Note. OnDestroyed is called from WM_NCDESTROY.

I have observed this too while working on Webview2. This is something we can explore.

@captainjono
Copy link
Owner Author

captainjono commented Nov 4, 2020

I have added a [CefBrowserHostView retrain] call in dealloc and it seems to be surpressing the crash in my testing thus far (could of caused a memory leak i know..). This is lending weight to the theory of double dispose caused by multiple calls to WindowDestroy().

Im dealing with other errors now I think which are artefacts of other systems in my app. I'll continue my testing and grep the reply i got to the issue i posed.

@captainjono
Copy link
Owner Author

This PR resolves the issue

#5

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

No branches or pull requests

2 participants