get_running_executor a modern alternative

Summary:
In the long term I want to force all usage of get_executor to require that the call comes from a running asyncio eventloop as its safer/saner than allowing the default un-running loop to be used since it may never actually run.  Until then I have created get_running_executor that I can use in various places. When get_running_executor(True) is used everywhere we can codemod it back to get_executor() and have asyncio.get_running_loop() used exclusively.

A folly::IOBuf can be built around a Python Buffer.  It supports three ways to ask python to cleanup that buffer.

1. If it has the GIL just decref the buffer
2. else if it has a Python Executor it can schedule the cleanup
3. it can rely on some cPython magic called "Py_AddPendingCall"

thrift-py3 deserialize wraps a python buffer in a IOBuf, but since ownership is not handed off to C++ it has the GIL when we destroy the IOBUF so it doesn't need an Executor.

IOBuf calls folly.executor.get_executor() which will create a AsyncioExecutor and bind it to the default eventloop.

So using deserialize right before a python fork, would cause a ABORT in the child since the AsyncioExecutor would try to drive() itself during its destruction.

The asyncio default eventloop is considered a bad thing, new interfaces like asyncio.run() does not rely on a default loop but creating its own.
To insure we only ever use the eventloop from a running loop, use get_running_loop.

Reviewed By: sdunster

Differential Revision: D26794162

fbshipit-source-id: 8e7e53c453c9ccff7e81f02e0692c8bebad70edd

folly/python/executor.cpp

@@ -37,7 +37,7 @@ void do_import() {
 
 folly::Executor* getExecutor() {
   FOLLY_MAYBE_UNUSED static bool done = (do_import(), false);
-  return get_executor();
+  return get_running_executor(false); // TODO: fried set this to true
 }
 
 } // namespace python

folly/python/executor.pxd

@@ -27,3 +27,4 @@ cdef class AsyncioExecutor:
     cdef unique_ptr[cAsyncioExecutor] cQ
 
 cdef api cAsyncioExecutor* get_executor()
+cdef api cAsyncioExecutor* get_running_executor(bint running)

folly/python/executor.pyx

@@ -42,9 +42,19 @@ cdef class AsyncioExecutor:
         self.cQ.reset()
 
 
-cdef cAsyncioExecutor* get_executor():
+# TODO: fried this is a stop gap, we really should not bind things to
+# the default eventloop if its not running. As it may never be run.
+# modern python asyncio suggests never using the "default" event loop
+# I don't believe we will be able to force the behavior that
+# get_executor() should always be run from a running eventloop in a single
+# diff. But ultimately we will want to remove this function and
+# go back to just get_executor() that only binds to a running loop.
+cdef cAsyncioExecutor* get_running_executor(bint running):
     try:
-        loop = asyncio.get_event_loop()
+        if running:
+            loop = asyncio.get_running_loop()
+        else:
+            loop = asyncio.get_event_loop()
     except RuntimeError:
         return NULL
     try:
@@ -54,3 +64,6 @@ cdef cAsyncioExecutor* get_executor():
         loop.add_reader(Q.fileno(), Q.drive)
         loop_to_q[loop] = Q
     return Q.cQ.get()
+
+cdef cAsyncioExecutor* get_executor():
+    return get_running_executor(False)

folly/python/iobuf.pyx

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 from builtins import memoryview as py_memoryview
-from folly.executor cimport get_executor
+from folly.executor cimport get_running_executor
 from cpython cimport Py_buffer
 from weakref import WeakValueDictionary
 from cpython.object cimport Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, Py_GE
@@ -29,7 +29,7 @@ cdef unique_ptr[cIOBuf] from_python_buffer(memoryview view):
         raise ValueError("View must be contiguous")
     return move(
         iobuf_from_python(
-            get_executor(),
+            get_running_executor(True),
             <PyObject*>view,
             view.view.buf,
             view.view.len,