WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 139 forks source link

Hard exit WebProcess with _exit() to skip exit handlers #1129

Closed asurdej-comcast closed 1 year ago

asurdej-comcast commented 1 year ago

There is a change in 2.38 that makes WebProcess exit cleaner by calling exit handlers (registered with atexit()) instead of using _exit() that skips handlers execution. Unfortunately this causes a crash inside NicosiaGCGLLayer.cpp:

terminateWindowContext()
~GLContextEGL():

    if (m_surface)
        eglDestroySurface(display, m_surface);

Surface destruction crashes as WPE compositor and native window are closed because UIProcess exited already.

This revert brings back wpe-2.28 behavior.

But there is another part of this issue. How to make sure that at the point of the UIProcess exit WebProcess (all child processes basically) are closed so it is safe to close compositor and clean up native parts? Currently we use g_clear_object(view) that destroys WebKitWebView obj but WebProcess seems to be running still https://github.com/rdkcentral/rdkservices/blob/sprint/23Q3/WebKitBrowser/WebKitImplementation.cpp#L2963

Do you have any suggestion here?

magomez commented 1 year ago

Nope, this is not the right way to fix this. Going back to the behavior of 2.28 is hiding the problem, not fixing it.

I think that the destruction of the WebProcess is an async operation, and the UIProcess doesn't block waiting for the WebProcess to be destroyed. If you need to ensure that the WebProcess is really destroyed before finishing the UIProcess, when we probably need to implement a sync close. I'll give a look to how this could be achieved and propose a patch.

asurdej-comcast commented 1 year ago

Fully agree, I didn't find any APIs to ensure synchronous WebProcess close. This would be needed to make sure it is safe to destroy compositor dedicated for particular browser instance. I can use some general unix mechanism but was hoping if there is something implemented already. Another thing is if compositor process is crashed or died for some reason. This case will be detected inside wpe backedn impl and would be nice to inform webprocess about it but I don't think we have anything like this

Anyway, I will mark this PR as a draft because the patch won't be merged probably

magomez commented 1 year ago

@asurdej-comcast I'll handle this issue in https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/1134 and close this.

asurdej-comcast commented 11 months ago

Related to #1230