Update to handling of browser and frame references (cztomczak#365)

cztomczak · cztomczak · commit 7aeee7733219 · 2018-08-02T16:41:30.000+02:00
There are still some strange issues with CEF. For example I can
see that OnContextReleased is executed twice for that same frame.
diff --git a/src/browser.pyx b/src/browser.pyx
@@ -74,7 +74,8 @@ cdef PyBrowser GetPyBrowser(CefRefPtr[CefBrowser] cefBrowser,
     cdef JavascriptBindings javascriptBindings
     cdef PyBrowser tempPyBrowser
 
-    if browserId in g_unreferenced_browsers:
+    if browserId in g_unreferenced_browsers \
+            or browserId in g_closed_browsers:
         # This browser was already unreferenced due to OnBeforeClose
         # was already called. An incomplete new instance of Browser
         # object is created. This instance doesn't have the client
@@ -119,9 +120,13 @@ cdef PyBrowser GetPyBrowser(CefRefPtr[CefBrowser] cefBrowser,
 cdef void RemovePyBrowser(int browserId) except *:
     # Called from LifespanHandler_OnBeforeClose().
     global g_pyBrowsers, g_unreferenced_browsers
+    cdef PyBrowser pyBrowser
     if browserId in g_pyBrowsers:
         # noinspection PyUnresolvedReferences
         Debug("del g_pyBrowsers[%s]" % browserId)
+        pyBrowser = g_pyBrowsers[browserId]
+        pyBrowser.cefBrowser.Assign(NULL)
+        del pyBrowser
         del g_pyBrowsers[browserId]
         g_unreferenced_browsers.append(browserId)
     else:
diff --git a/src/cefpython.pyx b/src/cefpython.pyx
@@ -843,11 +843,6 @@ def QuitMessageLoop():
 def Shutdown():
     Debug("Shutdown()")
 
-    # Release shared request context. This is sometimes causing
-    # segmentation fault, so disabling it for now. See Issue #333:
-    # https://github.com/cztomczak/cefpython/issues/333
-    # OFF: g_shared_request_context.Assign(NULL)
-
     # Run some message loop work, force closing browsers and then run
     # some message loop work again for the browsers to close cleanly.
     #
@@ -929,6 +924,12 @@ def Shutdown():
         NonCriticalError("Shutdown called, but there are still browser"
                          " references alive")
 
+    # Release shared request context. In the past this was sometimes
+    # causing segmentation fault. See Issue #333:
+    # https://github.com/cztomczak/cefpython/issues/333
+    # Debug("Free g_shared_request_context")
+    # g_shared_request_context.Assign(NULL)
+
     Debug("CefShutdown()")
     with nogil:
         CefShutdown()
diff --git a/src/frame.pyx b/src/frame.pyx
@@ -7,6 +7,10 @@ include "browser.pyx"
 
 cdef dict g_pyFrames = {}
 
+# If a frame was unreferenced (browser closed or OnContextReleased)
+# it shouldn't be kept global anymore.
+cdef list g_unreferenced_frames = []  # [str unique identifier, ..]
+
 cdef object GetUniqueFrameId(int browserId, object frameId):
     return str(browserId) +"#"+ str(frameId)
 
@@ -24,6 +28,11 @@ cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
 
     cdef PyFrame pyFrame
     cdef object frameId = cefFrame.get().GetIdentifier()  # int64
+    if frameId < 0:
+        # Underlying frame does not yet exist.
+        Debug("GetPyFrame(): underlying frame does not yet exist"
+              ", frameId = {0}".format(frameId))
+        return None
     cdef int browserId = cefFrame.get().GetBrowser().get().GetIdentifier()
     assert (frameId and browserId), "frameId or browserId empty"
     cdef object uniqueFrameId = GetUniqueFrameId(browserId, frameId)
@@ -46,9 +55,12 @@ cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
     pyFrame = PyFrame(browserId, frameId)
     pyFrame.cefFrame = cefFrame
 
-    if browserId in g_unreferenced_browsers:
+    if uniqueFrameId in g_unreferenced_frames \
+            or browserId in g_unreferenced_browsers \
+            or browserId in g_closed_browsers:
         # Browser was already globally unreferenced in OnBeforeClose,
-        # thus all frames are globally unreferenced too. Create a new
+        # thus all frames are globally unreferenced too, or frame
+        # was unreferenced in OnContextReleased. Create a new
         # incomplete instance of PyFrame object. Read comments in
         # browser.pyx > GetPyBrowser and in Browser.md for what
         # "incomplete" means.
@@ -60,17 +72,22 @@ cdef PyFrame GetPyFrame(CefRefPtr[CefFrame] cefFrame):
         # SIDE EFFECT: two calls to GetPyFrame for the same frame object
         #              may return two different PyFrame objects. Compare
         #              frame objects always using GetIdentifier().
-        # Debug("GetPyFrame(): create new PyFrame, frameId=%s" % frameId)
+        Debug("GetPyFrame(): create new PyFrame, frameId=%s" % frameId)
         g_pyFrames[uniqueFrameId] = pyFrame
     return pyFrame
 
 cdef void RemovePyFrame(int browserId, object frameId) except *:
     # Called from V8ContextHandler_OnContextReleased().
     global g_pyFrames
+    cdef PyFrame pyFrame
     cdef object uniqueFrameId = GetUniqueFrameId(browserId, frameId)
     if uniqueFrameId in g_pyFrames:
         Debug("del g_pyFrames[%s]" % uniqueFrameId)
+        pyFrame = g_pyFrames[uniqueFrameId]
+        pyFrame.cefFrame.Assign(NULL)
+        del pyFrame
         del g_pyFrames[uniqueFrameId]
+        g_unreferenced_frames.append(uniqueFrameId)
     else:
         Debug("RemovePyFrame() FAILED: uniqueFrameId = %s" % uniqueFrameId)
 
@@ -85,7 +102,11 @@ cdef void RemovePyFramesForBrowser(int browserId) except *:
             toRemove.append(uniqueFrameId)
     for uniqueFrameId in toRemove:
         Debug("del g_pyFrames[%s]" % uniqueFrameId)
+        pyFrame = g_pyFrames[uniqueFrameId]
+        pyFrame.cefFrame.Assign(NULL)
+        del pyFrame
         del g_pyFrames[uniqueFrameId]
+        g_unreferenced_frames.append(uniqueFrameId)
 
 cdef class PyFrame:
     cdef CefRefPtr[CefFrame] cefFrame
diff --git a/src/handlers/lifespan_handler.pyx b/src/handlers/lifespan_handler.pyx
@@ -110,6 +110,7 @@ cdef public void LifespanHandler_OnBeforeClose(
         CefRefPtr[CefBrowser] cefBrowser
         ) except * with gil:
     cdef PyBrowser pyBrowser
+    cdef int browserId
     cdef object callback
     try:
         Debug("LifespanHandler_OnBeforeClose")
@@ -126,14 +127,20 @@ cdef public void LifespanHandler_OnBeforeClose(
         callback = pyBrowser.GetClientCallback("OnBeforeClose")
         if callback:
             callback(browser=pyBrowser)
-        RemovePythonCallbacksForBrowser(pyBrowser.GetIdentifier())
-        RemovePyFramesForBrowser(pyBrowser.GetIdentifier())
-        RemovePyBrowser(pyBrowser.GetIdentifier())
+
+        browserId = pyBrowser.GetIdentifier()
+        pyBrowser.cefBrowser.Assign(NULL)
+        cefBrowser.Assign(NULL)
+        del pyBrowser
+
+        RemovePythonCallbacksForBrowser(browserId)
+        RemovePyFramesForBrowser(browserId)
+        RemovePyBrowser(browserId)
+
         if g_MessageLoop_called and not len(g_pyBrowsers):
             # Automatically quit message loop when last browser was closed.
             # This is required for hello_world.py example to work.
-            QuitMessageLoop()
-
+            PostTask(TID_UI, QuitMessageLoop)
     except:
         (exc_type, exc_value, exc_trace) = sys.exc_info()
         sys.excepthook(exc_type, exc_value, exc_trace)
