search

cefsharp / CefSharp

.NET (WPF and Windows Forms) bindings for the Chromium Embedded Framework
http://cefsharp.github.io/
Other
9.79k stars 2.92k forks source link

BrowserSubprocess - Update shutdown priority (SetProcessShutdownParameters) #3155

Closed ekalchev closed 2 years ago

ekalchev commented 4 years ago

Bug Report

Delete this line and everything above, and then fill in the details below.

amaitland commented 4 years ago

Generally speaking there has been some refactoring lately, nothing that I can think of that would influence this.

The default CefSharp.BrowserSubprocess.exe should terminate itself when the parent process exits.

https://github.com/cefsharp/CefSharp/blob/cefsharp/83/CefSharp.BrowserSubprocess.Core/BrowserSubprocessExecutable.h#L110 https://github.com/cefsharp/CefSharp/blob/cefsharp/83/CefSharp/Internals/ParentProcessMonitor.cs#L24

Is you application closing itself as expected???

  • We believe the problem is in browser subprocess

What testing have you done to confirm this exactly? Have you tried packaging the CEF Sample Application and confirming that it works as expected? It's possible that Chromium has changed it's behaviour.

ekalchev commented 4 years ago

Our testing was comparing CefSharp 73 with CefSharp 81. With Cef sharp 73 MSI installer shows a dialog with the processes it needs to close in order to complete the installation. v73 doesn't list BrowserSubprocess in this list. With v81 BrowserSubprocess is part of this list and it is first. I suspect MSI is sending shutdown signals or trying to kill BrowserSubprocess unsuccessfully. Probably MSI is trying to close the processes sequentially and since BrowserSubprocess is the first in the list and it can't close it it won't get to the actual app process that BrowserSubprocess is child to. I am looking for a change in the code where BrowserSubprocess was registered as child process to the main app process.

amaitland commented 4 years ago

I'd still suggest trying to package the cefclient sample application, you may need to go digging through the CEF/chromium source if the behaviour reproduces.

You can compare the source history for the CefSharp.BrowserSubprocess.Core and CefSharp.BrowserSubprocess projects. There's nothing I can think of that would be influencing the behaviour from a CefSharp point of view.

amaitland commented 4 years ago

You can probably include the cefclient.exe from the matching CEF distribution and use that as the browsersubprocess for a simpler test, you'll only be able to do basic browsing, that should be sufficient for testing purposes.

ekalchev commented 4 years ago

You can probably include the cefclient.exe from the matching CEF distribution and use that as the browsersubprocess for a simpler test, you'll only be able to do basic browsing, that should be sufficient for testing purposes.

Will try that.

no-response[bot] commented 4 years ago

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

GSonofNun commented 2 years ago

We recently realized this happens with our application and installer as well. From what I can gather, the installer asks Windows to shut down the application so it can update the assembly files. Windows uses the Restart Manager to query and instruct applications to close. According to the documentation, it will

  1. Send a WM_QUERYENDSESSION message to the application a. If the application returns false, it will cancel the shutdown request b. If the application returns true, it will continue
  2. Send a WM_ENDSESSION message to the application.
  3. If the application hasn't shut down after a few seconds, it will send a WM_CLOSE message, which will terminate the process.

Since the shutdown request seems to be canceled (step 1.a), that would lead me to believe that someone is returning false when they receive the WM_QUERYENDSESSION message. (However, I can't find anywhere in Cef or CefSharp's code that it is doing that)

If we don't create a ChromiumBrowser or initialize CefSharp, then our application will receive and respond appropriately to the messages. However, once Cef has been initialized, we no longer receive those particular messages, even though we continue receiving other standard window messages

I've been trying to fix this for a day and a half, and have gotten no closer to fixing it, other than gaining better knowledge on how it is supposed to work.

CEF4Delphi looks to have had a similar problem, but I wasn't able to get anything useful from their solution. I added a window message handler in ChromiumWebBrowser to receive the WM_QUERYENDSESSION and WM_ENDSESSION messages, but after testing and still not receiving the messages, I realized that I was just attaching to the event on the same window handle as my main application, so of course I wouldn't see anything different. (Also, I don't like their solution because it forces their application to close when receiving the WM_QUERYENDSESSION message, when it should really do it when it receives the WM_ENDSESSION message.

I apologize for the dump of information, if you have any ideas or questions, feel free to voice them.

salvadordf commented 2 years ago

Sorry for intruding in this project. The solution in CEF4Delphi was this : // Subprocesses will be the last to be notified about the Windows shutdown. // The main browser process will receive WM_QUERYENDSESSION before the subprocesses // and that allows to close the application in the right order. SetProcessShutdownParameters(0x100, SHUTDOWN_NORETRY);

Chromium also calls SetProcessShutdownParameters : https://source.chromium.org/chromium/chromium/src/+/main:chrome/app/main_dll_loader_win.cc?q=SetProcessShutdownParameters&ss=chromium

amaitland commented 2 years ago

Sorry for intruding in this project. The solution in CEF4Delphi was this :

@salvadordf Thanks, greatly appreciated.

If I understand correctly for the sub processes we should call SetProcessShutdownParameters with a low value.

All processes start at shutdown level 0x280.

As per https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-setprocessshutdownparameters

Chromium also calls SetProcessShutdownParameters : https://source.chromium.org/chromium/chromium/src/+/main:chrome/app/main_dll_loader_win.cc?q=SetProcessShutdownParameters&ss=chromium

Looking through the Chromium source there appears to be two different values used, 

//For crashpad
if (!SetProcessShutdownParameters(0x100, SHUTDOWN_NORETRY))
    PLOG(ERROR) << "SetProcessShutdownParameters";

//For all others
//The browser uses the default shutdown order, which is 0x280.
const int kNonBrowserShutdownPriority = 0x280;
::SetProcessShutdownParameters(kNonBrowserShutdownPriority - 1,
                                   SHUTDOWN_NORETRY);

@salvadordf Is using 0x100 for all processes working as expected for you? It appears that Chromium uses 0x100 only for crashpad and 0x280 - 1 for other subprocesses.

amaitland commented 2 years ago

// Shut down as late as possible relative to programs we're watching.

Based on the comment in the Chromium source I'd be inclined to set Crashpad to a value lower than the other sub processes.

salvadordf commented 2 years ago

@amaitland The current 0x100 value works fine but any value lower than the default should also work. I guess Chromium developers wanted to be sure the crashpad is closed last, even after the subprocesses.... but you're right, Even if we don't use the crashpad I should use the same value as Chromium for the subprocesses.

GSonofNun commented 2 years ago

@salvadordf Thank you for sharing that info!

@amaitland I'll pull that commit into my local CefSharp repo and give it a try. I'll check in later with the results.

GSonofNun commented 2 years ago

No luck. ☹️ Something is still preventing the main application from receiving the messages.

Also, the comments you left on your (@amaitland) commit say "Lower the shutdown priority so the browser process is shutdown first", but the documentation on the SetProcessShutdownParameters say:

The system shuts down processes from high dwLevel values to low.

So I think lowering the shutdown priority actually makes them shut down after the main process? With that in mind, I experimented by increasing the priority above 0x280 and still had the problem.

amaitland commented 2 years ago

So I think lowering the shutdown priority actually makes them shut down after the main process?

@GSonofNun Correct, this is the desired behaviour, matches that used by Chromium. Ideally your application (browser process) will shutdown gracefully and terminate the sub processes.

Shutting the sub process down first would likely cause chromium to attempt to relaunch them.

GSonofNun commented 2 years ago

Oh, I think I see my confusion. The "browser" referenced in your comments on the commit were referring to what would be the main application using CefSharp?

amaitland commented 2 years ago

Oh, I think I see my confusion. The "browser" referenced in your comments on the commit were referring to what would be the main application using CefSharp?

@GSonofNun Correct. See https://github.com/cefsharp/CefSharp/wiki/General-Usage#processes

Probably MSI is trying to close the processes sequentially and since BrowserSubprocess is the first in the list and it can't close it it won't get to the actual app process that BrowserSubprocess is child to. I am looking for a change in the code where BrowserSubprocess was registered as child process to the main app process.

As per https://github.com/cefsharp/CefSharp/issues/3155#issuecomment-652263660

The order should hopefully be changed, so the Browser Process (main application) is closed first. Subsequently the BrowserSubProcesses should terminate, they will forcefully close themselves when the Browser Process closes so they aren't left lingering if shutdown wasn't successful.

This should hopefully resolve the original issue raised by @ekalchev

Something is still preventing the main application from receiving the messages.

What exactly are you doing? What does your code look like? Not clear this is related to the original issue. Relevant events appear to be raised with a cursory test for me.

Using https://github.com/cefsharp/CefSharp.MinimalExample and rmlogotest.exe I can see that the FormClosing event is raised with a shutdown reason code and SystemEvents.SessionEnding event is fired.

GSonofNun commented 2 years ago

I did some more testing. We are listening to all windows messages and writing them to a log file. For reference, here's what it looks like.

private IntPtr WindowProc(IntPtr hwnd, int msg, IntPtr wParam, IntPtr lParam, ref bool handled)
{
    Logger.Log(EventLevel.Informational, $"Windows Message: {msg} {wParam} {lParam}");

    if (msg == WindowsMessages.WM_QUERYENDSESSION)
    {
        Logger.Log(EventLevel.Informational, "Windows Restart Manager: WM_QUERYENDSESSION message received");
        return IntPtr.Zero;
    }
    else if (msg == WindowsMessages.WM_ENDSESSION)
    {
        Logger.Log(EventLevel.Informational, "Windows Restart Manager: WM_ENDSESSION message received");
        if (wParam.ToInt64() == 0)
        {
            Logger.Log(EventLevel.Informational, "Windows Restart Manager: Canceling close");
        }
        else
        {
            Logger.Log(EventLevel.Informational, "Windows Restart Manager: Ending sessions");
            Environment.Exit(0);// Close Application TODO: Save user state
        }
        handled = true;
        return IntPtr.Zero;
    }

    return IntPtr.Zero;
}
amaitland commented 2 years ago

For reference, here's what it looks like.

Are you using WPF? Please provide more detail.

What is the reason for calling Environment.Exit? Doesn't WPF handle the messages gracefully?

Test 1: Debugging our application in Visual Studio and putting breakpoints in the message handler method.

How does it behave if you run directly from your bin folder?

If you create an installer for the minimum example how does it behave? What installer are you using? Just saying installer is vaigue.

https://github.com/cefsharp/CefSharp.MinimalExample

GSonofNun commented 2 years ago

Are you using WPF?

Sorry, yes. A WPF application using .NET 6.

What is the reason for calling Environment.Exit? Doesn't WPF handle the messages gracefully?

I don't think that matters. Removing that line makes no difference. It was just temporarily there to simulate our application saving state and shutting down.

How does it behave if you run directly from your bin folder?

I behaves like Test 2. I receive WM_QUERYENDSESSION, followed by WM_CLOSE, which closes the window but leaves the process "running" in the background.

What installer are you using?

We're using an installer generated by Wix

I'll try packaging the CefSharp.MinimalExample and get back to you.

GSonofNun commented 2 years ago

Well, my day has been disappointing. ☹️

I spent most of the day figuring out how to build the minimal example into an installer using Wix (I'm not super familiar with Wix, so I had to re-learn how it works). I finally got it to work, only to have results that disappoint me, but will probably please you. 🥲

While the installed version of the minimal example is running, if I use rmlogotest.exe, it closes perfectly, as it should.

However, while it is running, if I re-run the installer to repair it, and give it permission to shut down the running application, it fails.

Which leads me to wonder if it is a problem with Wix and a multi-process application. Maybe it isn't sending the shutdown message to the correct application. ¯\_(ツ)_/¯

amaitland commented 2 years ago

I spent most of the day figuring out how to build the minimal example into an installer using Wix (I'm not super familiar with Wix, so I had to re-learn how it works)

Can you fork the minimum example and push your changes to GitHub so I can have a look. Preferably squashed into a single commit. Thanks.

GSonofNun commented 2 years ago

Here you go. Let me know if you have any questions. https://github.com/GSonofNun/CefSharp.MinimalExample/tree/add-installer

GSonofNun commented 2 years ago

Update: Because of a change we're making to our solution, I remembered that we are building our application as a .NET 6 Self-Contained application. So I updated our application to self-host the browser subprocess, and now it responds to the installer's shutdown request as expected.

amaitland commented 2 years ago

Here you go. Let me know if you have any questions. https://github.com/GSonofNun/CefSharp.MinimalExample/tree/add-installer

@GSonofNun Thanks for the example.

So I updated our application to self-host the browser subprocess, and now it responds to the installer's shutdown request as expected.

I had a suspicion this might be the case, was on my list of cases to test. From what I can tell Wix doesn't like when there are two different exes running from the same folder. Using Spy++ to check the messages sent to the hwnd and I don't a WM_QUERYENDSESSION message.

After much digging and experimentation calling WixCloseApplications earlier in the installer sequence works just fine. I've pushed an updated version of your example to https://github.com/cefsharp/CefSharp.MinimalExample/tree/wix-installer

I don't know enough about Wix to know exactly what's going on, if anyone is interested in following up with the Wix Team then please post a link back here for reference.

A quick example using https://github.com/oleg-shilo/wixsharp to package the WinForms version for reference.

using System;
using WixSharp;
using Microsoft.Deployment.WindowsInstaller;
using System.IO;

namespace CefSharp.MinimalExample.WinForms.WixSharp
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            var bitness = Environment.Is64BitProcess ? "x64" : "x86";
#if DEBUG
            const string Build = "Debug";
#else
            const string Build = "Release";
#endif
            var buildPath = Path.GetFullPath(@"..\CefSharp.MinimalExample.WinForms\bin.net472\" + bitness + @"\" + Build + @"\net472");
            var project = new Project("CefSharp.MinimalExample.WinForms",
                          new Dir(@"%ProgramFiles%\CefSharp.MinimalExample.WinForms",
                              new DirFiles($"{buildPath}\\*.*"),
                              new Dir("locales", new DirFiles($"{buildPath}\\locales\\*.*")),
                              new Dir("swiftshader", new DirFiles($"{buildPath}\\swiftshader\\*.*"))),
                          new CustomActionRef("WixCloseApplications", When.Before, Step.CostFinalize, new Condition("VersionNT > 400")),
                          new CloseApplication(new Id("CefSharp.MinimalExample.WinForms"), "CefSharp.MinimalExample.WinForms.net472.exe")
                          {
                              Timeout = 15,
                              EndSessionMessage = true,
                              RebootPrompt = false,
                          },
                          new Property("MsiLogging", "vocewarmup"));

            project.GUID = new Guid("9007D203-B084-482F-BC23-5F4BF82EE679");
            project.Platform = Platform.x64;

            Compiler.BuildMsi(project);
        }
    }
}

I'm going to close this issue now. If someone contacts the Wix Team and it's determined that further code changes are required then it's probably best to create a new issue and reference this one.

amaitland commented 2 years ago

See https://github.com/cefsharp/CefSharp/issues/3407#issuecomment-782828504 for example of self hosting the browser sub process (using your own applications exe instead of CefSharp.BrowserSubprocess.exe).