CefSharp icon indicating copy to clipboard operation
CefSharp copied to clipboard
verified publisher icon cefsharp

WinForms/WPF/OffScreen - Object reference incorrectly held at CefShutdown

Open kiewic opened this issue 6 years ago • 9 comments

  • What version of the product are you using?
    • Nuget CefSharp.OffScreen version 79.1.35
    • CEF cef_binary_79.1.35+gfebbb4a+chromium-79.0.3945.130_windows64 debug binaries
  • What architecture x86 or x64?
    • At least in x64,
    • Though I was able to reproduce with x86 version 75.1.14
  • On what operating system?
    • Win10
  • Are you using WinForms, WPF or OffScreen?
    • OffScreen
  • What steps will reproduce the problem?
    • Using OffScreen, load a page, and as soon as it completes loading, call Cef.Shutdown()
    • Minimal repro source code is at https://github.com/kiewic/reference-incorrectly-held-at-CefShutdown/tree/version-79
    • Debug CEF binaries are necessary, since the CEF's shutdown_checker implementation is empty in Release versions
  • What is the expected output? What do you see instead?
    • No crash or debug.log errors are expected. However, the process crashes [almost every time] after printing the following message:
[0208/223134.848:FATAL:shutdown_checker.cc(52)] Check failed: !IsCefShutdown(). Object reference incorrectly held at CefShutdown
[0208/223134.887:FATAL:shutdown_checker.cc(52)] Check failed: !IsCefShutdown(). Object reference incorrectly held at CefShutdown
  • Please provide any additional information below.

The crash callstack is:

00 libcef!logging::LogMessage::~LogMessage+0x647 [Y:\work\CEF3_git\chromium\src\base\logging.cc @ 954] 
01 libcef!shutdown_checker::AssertNotShutdown+0x78 [Y:\work\CEF3_git\chromium\src\cef\libcef_dll\shutdown_checker.cc @ 54] 
02 libcef!CefLifeSpanHandlerCToCpp::OnBeforeClose+0x29 [Y:\work\CEF3_git\chromium\src\cef\libcef_dll\ctocpp\life_span_handler_ctocpp.cc @ 150] 
03 libcef!CefBrowserHostImpl::DestroyBrowser+0xf3 [Y:\work\CEF3_git\chromium\src\cef\libcef\browser\browser_host_impl.cc @ 1539] 
04 libcef!CefBrowserInfoManager::DestroyAllBrowsers+0x13f [Y:\work\CEF3_git\chromium\src\cef\libcef\browser\browser_info_manager.cc @ 332] 
05 libcef!CefContext::FinishShutdownOnUIThread+0x7a [Y:\work\CEF3_git\chromium\src\cef\libcef\browser\context.cc @ 606] 
06 libcef!base::OnceCallback<void ()>::Run+0x61 [Y:\work\CEF3_git\chromium\src\base\callback.h @ 98] 
07 libcef!base::TaskAnnotator::RunTask+0x185 [Y:\work\CEF3_git\chromium\src\base\task\common\task_annotator.cc @ 142] 
08 libcef!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl+0x1b5 [Y:\work\CEF3_git\chromium\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc @ 366] 
09 libcef!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoSomeWork+0x61 [Y:\work\CEF3_git\chromium\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc @ 221] 
0a libcef!base::MessagePumpForUI::DoRunLoop+0x143 [Y:\work\CEF3_git\chromium\src\base\message_loop\message_pump_win.cc @ 219] 
0b libcef!base::MessagePumpWin::Run+0xa4 [Y:\work\CEF3_git\chromium\src\base\message_loop\message_pump_win.cc @ 77] 
0c libcef!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run+0x129 [Y:\work\CEF3_git\chromium\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc @ 467] 
0d libcef!base::RunLoop::Run+0x30e [Y:\work\CEF3_git\chromium\src\base\run_loop.cc @ 158] 
0e libcef!CefUIThread::ThreadMain+0x77 [Y:\work\CEF3_git\chromium\src\cef\libcef\common\main_delegate.cc @ 395] 
0f libcef!base::`anonymous namespace'::ThreadFunc+0xcc [Y:\work\CEF3_git\chromium\src\base\threading\platform_thread_win.cc @ 105] 
10 KERNEL32!BaseThreadInitThunk+0x14 [base\win32\client\thread.c @ 64] 
11 ntdll!RtlUserThreadStart+0x21 [minkernel\ntdll\rtlstrt.c @ 1153]

But the main thread (thread 0) has already called CefShutdown:

00 ntdll!ZwWaitForSingleObject+0x14 [minkernel\ntdll\daytona\objfre\amd64\usrstubs.asm @ 211] 
01 KERNELBASE!WaitForSingleObjectEx+0x93 [minkernel\kernelbase\synch.c @ 1328] 
02 libcef!base::WaitableEvent::Wait+0xab [Y:\work\CEF3_git\chromium\src\base\synchronization\waitable_event_win.cc @ 69] 
03 libcef!CefContext::Shutdown+0x10f [Y:\work\CEF3_git\chromium\src\cef\libcef\browser\context.cc @ 481] 
04 libcef!CefShutdown+0xb5 [Y:\work\CEF3_git\chromium\src\cef\libcef\browser\context.cc @ 272] 
05 CefSharp_Core!DomainBoundILStubClass.IL_STUB_PInvoke()+0x68
06 CefSharp_Core!CefSharp::Cef::Shutdown+0x283 [c:\projects\cefsharp\cefsharp.core\cef.h @ 89] 
07 CefSharpOffscreenRepro!CefSharpOffscreenRepro.Program.Main(System.String[])+0x28b*** WARNING: Unable to verify checksum for D:\repos\reference-incorrectly-held-at-CefShutdown\CefSharpOffscreenRepro\bin\x64\Debug\CefSharpOffscreenRepro.exe
 [D:\repos\reference-incorrectly-held-at-CefShutdown\CefSharpOffscreenRepro\Program.cs @ 64] 
08 CefSharpOffscreenRepro!CefSharpOffscreenRepro.Program.Main(System.String[])+0x1ae
09 clr!CallDescrWorkerInternal+0x83 [f:\dd\ndp\clr\src\vm\amd64\CallDescrWorkerAMD64.asm @ 97] 
0a clr!CallDescrWorkerWithHandler+0x4e [f:\dd\ndp\clr\src\vm\callhelpers.cpp @ 89] 
0b clr!MethodDescCallSite::CallTargetWorker+0x102 [f:\dd\ndp\clr\src\vm\callhelpers.cpp @ 655] 
0c clr!MethodDescCallSite::Call_RetArgSlot+0x5 [f:\dd\ndp\clr\src\vm\callhelpers.h @ 423] 
0d clr!RunMain+0x266 [f:\dd\ndp\clr\src\vm\assembly.cpp @ 2659] 
0e clr!Assembly::ExecuteMainMethod+0xb7 [f:\dd\ndp\clr\src\vm\assembly.cpp @ 2780] 
0f clr!SystemDomain::ExecuteMainMethod+0x643 [f:\dd\ndp\clr\src\vm\appdomain.cpp @ 3755] 
10 clr!ExecuteEXE+0x3f [f:\dd\ndp\clr\src\vm\ceemain.cpp @ 3053] 
11 clr!_CorExeMainInternal+0xb2 [f:\dd\ndp\clr\src\vm\ceemain.cpp @ 2887] 
12 clr!_CorExeMain+0x14 [f:\dd\ndp\clr\src\vm\ceemain.cpp @ 2814] 
13 mscoreei!_CorExeMain+0x112 [f:\dd\ndp\clr\src\dlls\shim\shim.cpp @ 6425] 
14 MSCOREE!_CorExeMain_Exported+0x6c [onecore\com\netfx\windowsbuilt\shell_shim\v2api.cpp @ 1223] 
15 KERNEL32!BaseThreadInitThunk+0x14 [base\win32\client\thread.c @ 64] 
16 ntdll!RtlUserThreadStart+0x21 [minkernel\ntdll\rtlstrt.c @ 1153]

You can download a mini dump of the crash here.

I have already provided the relevant log errors above, but you can find the full debug.log here.

Even when the repro project has a hardcoded URI, the issue reproduces with any URI.

  • Does this problem also occur in the CEF Sample Application
    • N/A

kiewic avatar Feb 09 '20 09:02 kiewic

What version of the product are you using?

  • Nuget CefSharp.OffScreen version 75.1.143
  • CEF cef_binary_79.1.35+gfebbb4a+chromium-79.0.3945.130_windows64 debug binaries

To be clear, you have or haven't tested with version 79.1.350?

amaitland avatar Feb 09 '20 09:02 amaitland

  • Minimal repro source code is at https://github.com/kiewic/reference-incorrectly-held-at-CefShutdown/tree/version-79

Just an FYI out of the box I cannot run your example because of https://bitbucket.org/chromiumembedded/cef/issues/2767/fatal-thread_restrictionscc-check-failed

amaitland avatar Feb 09 '20 10:02 amaitland

To be clear, you have or haven't tested with version 79.1.350?

I'm sorry, I meant version 79.1.350. Yes, I tested 79.1.350.

Just an FYI out of the box I cannot run your example because of https://bitbucket.org/chromiumembedded/cef/issues/2767/fatal-thread_restrictionscc-check-failed

I am not getting any error related to Check failed in debug (Windows) when missing call to CefEnableHighDPISupport 🤔

kiewic avatar Feb 09 '20 16:02 kiewic

I am not getting any error related to Check failed in debug (Windows) when missing call to CefEnableHighDPISupport

A call to Cef.EnableHighDPISupport(); (or the relevant app.manifest entries) is required for me to run your example using a debug build on Win 10 1903 x64 with my display set to 150%.

Looking at the example, are all the extra BrowserSettings, RequestContext, CefSettings required to reproduce the problem? Is the code the absolute minimum required to reproduce the problem?

amaitland avatar Feb 10 '20 23:02 amaitland

Looks like the example can be greatly simplified, the following should be sufficient to reproduce.

public static int Main(string[] args)
{
	Cef.EnableHighDPISupport();

	var manualResetEvent = new ManualResetEvent(false);

	var browser = new ChromiumWebBrowser("https://google.com");
	browser.LoadingStateChanged += (sender, e) =>
	{
		if (!e.IsLoading)
		{
			manualResetEvent.Set();
		}
	};

	manualResetEvent.WaitOne();

	browser.Dispose();

	Cef.Shutdown();

	return 0;
}

Adding a Thread.Sleep(500); before calling Cef.Shutdown should workaround the issue for now.

amaitland avatar Feb 11 '20 00:02 amaitland

When using MultiThreadedMessageLoop the call to Cef.Shutdown happens on the main application thread, the CEF UI thread is used for browser related cleanup including CefLifespanHandler::OnBeforeClose which in this case is being called after CefShutdown.

CefShutdown needs to be called after all CefBrowser instances have been closed.

I've added a new Cef.WaitForBrowsersToClose(); method which waits until the last CefLifespanHandler::OnBeforeClose call plus a small Thread.Sleep to allow for the extra code that's run after OnBeforeClose. The only method of checking if close was called successfully ('CefBrowserHost.TryCloseBrowser') requires being called on the CEF UI thread, I cannot at the moment think of a way to use this practically without blocking the Thread.

The Cef.Shutdown method currently disposes of any lingering ChromiumWebBrowser instances, with the new Shutdown checker that's going to be problematic (was probably always a little problematic, we just weren't being told about it).

  • Cef.Shutdown should either remove the loop
  • Cef.Shutdown should wait for all browser instances to close.

For now I'll leave the code as is until the Cef.WaitForBrowsersToClose(); method has had more testing. There's only a single Unit Test at the moment, will need to spawn a larger number of browser instances and confirm that CefShutdown is called without incident. Code needs a refactor as it's using a static instance (that'll hopefully happen as part of some other refactoring I've got planned).

It's possible the CefClient destructor gets called later than CefLifespanHandler::OnBeforeClose, so that's something else to look into.

amaitland avatar Feb 14 '20 10:02 amaitland

Additional improvements to the WPF control can likely be made, possibly the WinForms one as well.

Currently in WPF we hook Application.Exit to ensure Cef.Shutdown is called. Disposing of the ChromiumWebBrowser instances in say Dispatcher.ShutdownStarted then later calling Cef.Shutdown in Application.Exit will hopefully give CEF sufficient time to cleanup.

The WinForms control is less of an issue as WinForms automatically calls Dispose on each ChromiumWebBrowser instance as part of it's cleanup.

amaitland avatar Apr 17 '20 01:04 amaitland

For WinForms and WPF you can set CefSharpSettings.ShutdownOnExit to false prior to creating the first ChromiumWebBrowser instance to implement your own custom shutdown behaviour if the default is not suitable.

WinForms

No changes made to the default Application exit closing, the browser instances should already have been Disposed as part of the Form being closed.

You can optionally use Cef.WaitForBrowsersToClose(); in your application if the default behaviour isn't working correctly.

WPF

If the Dispatcher exists for the Thread.CurrentThread then the ShutdownStarted and ShutdownFinished events will be used. If no Dispatcher then the previous behaviour of using the Application.Exit event will be used.

  • Cef.PreShutdown called in Dispatcher.ShutdownStarted to dispose of the ChromiumWebBrowser instances
  • Cef.Shutdown called in Dispatcher.ShutdownFinished, the browser instances were freed earlier and should have sufficient time to have freed their resources.

We could additionally call Cef.WaitForBrowsersToClose(); in Dispatcher.ShutdownStarted if this change isn't sufficient.

If your application uses multiple Dispatchers then you may need to disable the default shutdown handling (details above) and implement your own.

OffScreen

The Cef.WaitForBrowsersToClose(); feature requires that you explicitly enable. After further testing will be enabled by default.

Cef.EnableWaitForBrowsersToClose();

Cef.Initialize(new CefSettings());

Cef.WaitForBrowsersToClose();

Cef.Shutdown();

It's possible to implement your own version of IBrowserRefCounter if desired for debugging/testing purposes. See https://github.com/cefsharp/CefSharp/commit/6b440a84a3652ed0d4f5a299b566e731dae9aa4c for how it's implemented currently.

Further work This issue will likely be closed and a new issue created to track the additional work required.

  • Write unit tests
  • Improve naming of internal objects
  • Enable by default
  • Refactor BrowserRefCounter.Instance static property (reference to IBrowserRefCounter should be passed into each class rather than a global static property).

amaitland avatar May 04 '20 01:05 amaitland

For .net core/.net 5 we can possibly implement IAsyncDisposable to wait for the CEF browser to be closed before signaling dispose is complete.

https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync

amaitland avatar May 29 '21 20:05 amaitland