Fix race condition in loop destruction

Summary:
See docblock in new test. If the FiberManager weakref is created after the AsyncioExecutor weakref, then the native FiberManager object may be destroyed without being drained.

Unfortunately, there's no way to enforce an ordering between the destructors here (since weakrefs are cleaned up in inverse order of creation).

This diff adds a `__dealloc__` method to AsyncioExecutor to drain the native FiberManager

Reviewed By: yfeldblum

Differential Revision: D25797920

fbshipit-source-id: d1a548ebd67e7d9153eb43b169bf009a6a986e68

folly/python/fiber_manager.pxd

+# Copyright (c) Facebook, Inc. and its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 from libcpp.memory cimport unique_ptr
 from folly.executor cimport cAsyncioExecutor
 
@@ -10,6 +24,7 @@ cdef extern from "folly/fibers/FiberManagerInternal.h" namespace "folly::fibers"
         pass
     cdef cppclass cFiberManager "folly::fibers::FiberManager":
         cFiberManager(unique_ptr[cLoopController], const cFiberManagerOptions&)
+        void loopUntilNoReady()
 
 cdef extern from "folly/fibers/ExecutorLoopController.h" namespace "folly::fibers":
     cdef cppclass cAsyncioLoopController "folly::fibers::ExecutorLoopController"(cLoopController):

folly/python/fiber_manager.pyx

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import asyncio
+from cython.operator cimport dereference as deref
 from libcpp.memory cimport unique_ptr
 from folly.executor cimport get_executor
 from folly.fiber_manager cimport (
@@ -36,6 +37,14 @@ cdef class FiberManager:
                 get_executor())),
             opts));
 
+    def __dealloc__(FiberManager self):
+        # drive one last time
+        deref(self.cManager).loopUntilNoReady()
+        # Explicitly reset here, otherwise it is possible
+        # that self.cManager dstor runs after python finalizes
+        # Cython deletes these after __dealloc__ returns.
+        self.cManager.reset()
+
 
 cdef cFiberManager* get_fiber_manager(const cFiberManagerOptions& opts):
    loop = asyncio.get_event_loop()

folly/python/test/teardown.py

+#!/usr/bin/env python3
+# Copyright (c) Facebook, Inc. and its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import unittest
+
+from . import simplebridge
+
+
+class Teardown(unittest.TestCase):
+    """
+    The lifetimes of the native AsyncioExecutor/FiberManager objects
+    are bound to that of the python event loop.
+
+    If the loop is destroyed with pending work in the fiber-manager,
+    there may be a race condition where the fiber manager is destroyed
+    before being drained.
+
+    These tests ensure that both objects are cleanly driven and destroyed
+    before the process exits, irrespective of the order in which these
+    objects are destroyed.
+    """
+
+    def test_fiber_manager_tear_down(self):
+        simplebridge.get_value_x5_semifuture(1)
+        simplebridge.get_value_x5_fibers(1)