Commit ad6c9640 authored by Jason Madden's avatar Jason Madden

Don't let destroying the default C loop from two different Python loop objects...

Don't let destroying the default C loop from two different Python loop objects crash the process. Fixes #1098
parent 6261a749
......@@ -58,6 +58,13 @@
order, or be a dotted name; it may also be assigned to an
object in Python code at ``gevent.config.loop``).
- Fix an interpreter crash that could happen if two or more ``loop``
objects referenced the default event loop and one of them was
destroyed and then the other one destroyed or (in the libev C
extension implementation only) deallocated (garbage collected). See
:issue:`1098`.
1.3a1 (2018-01-27)
==================
......
......@@ -312,6 +312,13 @@ class AbstractLoop(object):
starting_timer_may_update_loop_time = False
# Subclasses should set this in __init__ to reflect
# whether they were the default loop.
_default = None
# A class variable.
_default_loop_destroyed = False
def __init__(self, ffi, lib, watchers, flags=None, default=None):
self._ffi = ffi
self._lib = lib
......@@ -470,12 +477,32 @@ class AbstractLoop(object):
raise NotImplementedError()
def destroy(self):
"""
Clean up resources used by this loop.
Note that, unlike the libev C loop implementation, this object
*does not* have a finalizer (``__del__``). If you create loops
(especially loops that are not the default) you *should* call
this method when you are done with the loop.
"""
if self._ptr:
self._stop_aux_watchers()
try:
if self._default:
if not self._can_destroy_default_loop():
return False
type(self)._default_loop_destroyed = True
self._stop_aux_watchers()
finally:
# not ffi.NULL, we don't want something that can be
# passed to C and crash later. This will create nice friendly
# TypeError from CFFI.
self._ptr = None
# not ffi.NULL, we don't want something that can be
# passed to C and crash later.
self._ptr = None
return True
def _can_destroy_default_loop(self):
return not type(self)._default_loop_destroyed
@property
def ptr(self):
......@@ -566,7 +593,7 @@ class AbstractLoop(object):
@property
def default(self):
pass
return self._default if self._ptr else False
@property
def iteration(self):
......
......@@ -389,9 +389,15 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
## data members
cdef bint starting_timer_may_update_loop_time
# We must capture the 'default' state at initialiaztion
# time. Destroying the default loop in libev sets
# the libev internal pointer to 0, and ev_is_default_loop will
# no longer work.
cdef bint _default
def __cinit__(self, object flags=None, object default=None, libev.intptr_t ptr=0):
self.starting_timer_may_update_loop_time = 0
self._default = 0
libev.ev_prepare_init(&self._prepare,
<void*>gevent_run_callbacks)
libev.ev_timer_init(&self._periodic_signal_checker,
......@@ -405,6 +411,7 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
cdef object old_handler = None
if ptr:
self._ptr = <libev.ev_loop*>ptr
self._default = libev.ev_is_default_loop(self._ptr)
else:
c_flags = _flags_to_int(flags)
_check_flags(c_flags)
......@@ -415,6 +422,7 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
if _default_loop_destroyed:
default = False
if default:
self._default = 1
self._ptr = libev.gevent_ev_default_loop(c_flags)
if not self._ptr:
raise SystemError("ev_default_loop(%s) failed" % (c_flags, ))
......@@ -469,31 +477,48 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
finally:
self.starting_timer_may_update_loop_time = False
def _stop_watchers(self):
cdef _stop_watchers(self, libev.ev_loop* ptr):
if not ptr:
return
if libev.ev_is_active(&self._prepare):
libev.ev_ref(self._ptr)
libev.ev_prepare_stop(self._ptr, &self._prepare)
libev.ev_ref(ptr)
libev.ev_prepare_stop(ptr, &self._prepare)
if libev.ev_is_active(&self._periodic_signal_checker):
libev.ev_ref(self._ptr)
libev.ev_timer_stop(self._ptr, &self._periodic_signal_checker)
libev.ev_ref(ptr)
libev.ev_timer_stop(ptr, &self._periodic_signal_checker)
def destroy(self):
global _default_loop_destroyed
if self._ptr:
self._stop_watchers()
cdef libev.ev_loop* ptr = self._ptr
self._ptr = NULL
if ptr:
if self._default and _default_loop_destroyed:
# Whoops! Program error. They destroyed the default loop,
# using a different loop object. Our _ptr is still
# valid, but the libev loop is gone. Doing anything
# else with it will likely cause a crash.
return
self._stop_watchers(ptr)
if __SYSERR_CALLBACK == self._handle_syserr:
set_syserr_cb(None)
if libev.ev_is_default_loop(self._ptr):
if self._default:
_default_loop_destroyed = True
libev.ev_loop_destroy(self._ptr)
self._ptr = NULL
libev.ev_loop_destroy(ptr)
def __dealloc__(self):
if self._ptr:
self._stop_watchers()
if not libev.ev_is_default_loop(self._ptr):
libev.ev_loop_destroy(self._ptr)
self._ptr = NULL
cdef libev.ev_loop* ptr = self._ptr
self._ptr = NULL
if ptr != NULL:
if self._default and _default_loop_destroyed:
# See destroy(). This is a bug in the caller.
return
self._stop_watchers(ptr)
if not self._default:
libev.ev_loop_destroy(ptr)
@property
def ptr(self):
......@@ -578,8 +603,9 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
@property
def default(self):
_check_loop(self)
return True if libev.ev_is_default_loop(self._ptr) else False
# If we're destroyed, we are not the default loop anymore,
# as far as Python is concerned.
return self._default if self._ptr else False
@property
def iteration(self):
......@@ -662,7 +688,7 @@ cdef public class loop [object PyGeventLoopObject, type PyGeventLoop_Type]:
if not self._ptr:
return 'destroyed'
cdef object msg = self.backend
if self.default:
if self._default:
msg += ' default'
msg += ' pending=%s' % self.pendingcnt
msg += self._format_details()
......
......@@ -194,9 +194,6 @@ def embeddable_backends():
def time():
return libev.ev_time()
_default_loop_destroyed = False
from gevent._ffi.loop import AbstractLoop
......@@ -216,6 +213,8 @@ class loop(AbstractLoop):
def __init__(self, flags=None, default=None):
AbstractLoop.__init__(self, ffi, libev, _watchers, flags, default)
self._default = True if libev.ev_is_default_loop(self._ptr) else False
def _init_loop(self, flags, default):
c_flags = _flags_to_int(flags)
......@@ -224,7 +223,7 @@ class loop(AbstractLoop):
c_flags |= libev.EVFLAG_FORKCHECK
if default is None:
default = True
if _default_loop_destroyed:
if loop._default_loop_destroyed:
default = False
if default:
......@@ -272,17 +271,16 @@ class loop(AbstractLoop):
self.ref() # we should go through the loop now
def destroy(self):
global _default_loop_destroyed
if self._ptr:
ptr = self._ptr
super(loop, self).destroy()
should_destroy_loop = super(loop, self).destroy()
if globals()["__SYSERR_CALLBACK"] == self._handle_syserr:
set_syserr_cb(None)
if libev.ev_is_default_loop(ptr):
_default_loop_destroyed = True
libev.ev_loop_destroy(ptr)
if should_destroy_loop:
libev.ev_loop_destroy(ptr)
@property
def MAXPRI(self):
......@@ -329,10 +327,6 @@ class loop(AbstractLoop):
def __repr__(self):
return '<%s at 0x%x %s>' % (self.__class__.__name__, id(self), self._format())
@property
def default(self):
return True if libev.ev_is_default_loop(self._ptr) else False
@property
def iteration(self):
return libev.ev_iteration(self._ptr)
......
......@@ -86,6 +86,7 @@ class loop(AbstractLoop):
self._io_watchers = dict()
self._fork_watchers = set()
self._pid = os.getpid()
self._default = self._ptr == libuv.uv_default_loop()
def _init_loop(self, flags, default):
if default is None:
......@@ -95,6 +96,8 @@ class loop(AbstractLoop):
# closed.
if default:
# XXX: If the default loop had been destroyed, this
# will create a new one, but we won't destroy it
ptr = libuv.uv_default_loop()
else:
ptr = libuv.uv_loop_new()
......@@ -102,6 +105,10 @@ class loop(AbstractLoop):
if not ptr:
raise SystemError("Failed to get loop")
# Track whether or not any object has destroyed
# this loop. See _can_destroy_default_loop
ptr.data = ptr
return ptr
_signal_idle = None
......@@ -258,10 +265,15 @@ class loop(AbstractLoop):
def destroy(self):
if self._ptr:
ptr = self._ptr
super(loop, self).destroy()
should_destroy = super(loop, self).destroy()
ptr.data = ffi.NULL
assert self._ptr is None
libuv.uv_stop(ptr)
if not should_destroy:
# The default loop has already been destroyed.
# libuv likes to abort() the process in this case.
return
closed_failed = libuv.uv_loop_close(ptr)
if closed_failed:
assert closed_failed == libuv.UV_EBUSY
......@@ -284,8 +296,14 @@ class loop(AbstractLoop):
closed_failed = libuv.uv_loop_close(ptr)
assert closed_failed == 0, closed_failed
# XXX: Do we need to uv_loop_delete the non-default loop?
# Probably...
def _can_destroy_default_loop(self):
# We're being asked to destroy a loop that's,
# at the time it was constructed, was the default loop.
# If loop objects were constructed more than once,
# it may have already been destroyed, though.
# We track this in the data member.
return self._ptr.data
def debug(self):
"""
......@@ -391,10 +409,6 @@ class loop(AbstractLoop):
def update_now(self):
libuv.uv_update_time(self._ptr)
@property
def default(self):
return self._ptr == libuv.uv_default_loop()
def fileno(self):
if self._ptr:
fd = libuv.uv_backend_fd(self._ptr)
......
from __future__ import absolute_import, print_function
import gevent
import unittest
class TestDestroyHub(unittest.TestCase):
def test_destroy_hub(self):
# Loop of initial Hub is default loop.
hub = gevent.get_hub()
self.assertTrue(hub.loop.default)
# Loop of initial Hub is default loop.
hub = gevent.get_hub()
assert hub.loop.default, hub
# Save `gevent.core.loop` object for later comparison.
initloop = hub.loop
# Save `gevent.core.loop` object for later comparison.
initloop = hub.loop
# Increase test complexity via threadpool creation.
# Implicitly creates fork watcher connected to the current event loop.
tp = hub.threadpool
self.assertIsNotNone(tp)
# Increase test complexity via threadpool creation.
# Implicitly creates fork watcher connected to the current event loop.
tp = hub.threadpool
# Destroy hub. Does not destroy libev default loop if not explicitly told to.
hub.destroy()
# Destroy hub. Does not destroy libev default loop if not explicitly told to.
hub.destroy()
# Create new hub. Must re-use existing libev default loop.
hub = gevent.get_hub()
self.assertTrue(hub.loop.default)
# Create new hub. Must re-use existing libev default loop.
hub = gevent.get_hub()
assert hub.loop.default, hub
# Ensure that loop object is identical to the initial one.
self.assertIs(hub.loop, initloop)
# Ensure that loop object is identical to the initial one.
assert hub.loop is initloop
# Destroy hub including default loop.
hub.destroy(destroy_loop=True)
# Destroy hub including default loop.
hub.destroy(destroy_loop=True)
# Create new hub and explicitly request creation of a new default loop.
hub = gevent.get_hub(default=True)
self.assertTrue(hub.loop.default)
# Create new hub and explicitly request creation of a new default loop.
hub = gevent.get_hub(default=True)
assert hub.loop.default, hub
# `gevent.core.loop` objects as well as libev loop pointers must differ.
self.assertIsNot(hub.loop, initloop)
self.assertIsNot(hub.loop.ptr, initloop.ptr)
self.assertNotEqual(hub.loop.ptr, initloop.ptr)
# `gevent.core.loop` objects as well as libev loop pointers must differ.
assert hub.loop is not initloop
assert hub.loop.ptr != initloop.ptr
# Destroy hub including default loop, create new hub with non-default loop.
hub.destroy(destroy_loop=True)
hub = gevent.get_hub()
if not getattr(hub.loop, 'DEFAULT_LOOP_REGENERATES', False):
self.assertFalse(hub.loop.default)
else:
self.assertTrue(hub.loop.default)
# Destroy hub including default loop, create new hub with non-default loop.
hub.destroy(destroy_loop=True)
hub = gevent.get_hub()
if not getattr(hub.loop, 'DEFAULT_LOOP_REGENERATES', False):
assert not hub.loop.default, hub
else:
assert hub.loop.default
hub.destroy()
hub.destroy()
if __name__ == '__main__':
unittest.main()
from __future__ import print_function
import gevent
import unittest
class TestDestroyDefaultLoop(unittest.TestCase):
def test_destroy_gc(self):
# Issue 1098: destroying the default loop
# while using the C extension could crash
# the interpreter when it exits
# Create the hub greenlet. This creates one loop
# object pointing to the default loop.
gevent.get_hub()
# Get a new loop object, but using the default
# C loop
loop = gevent.config.loop(default=True)
self.assertTrue(loop.default)
# Destroy it
loop.destroy()
# It no longer claims to be the default
self.assertFalse(loop.default)
# Delete it
del loop
# Delete the hub. This prompts garbage
# collection of it and its loop object.
# (making this test more repeatable; the exit
# crash only happened when that greenlet object
# was collected at exit time, which was most common
# in CPython 3.5)
del gevent.hub._threadlocal.hub
def test_destroy_two(self):
# Get two new loop object, but using the default
# C loop
loop1 = gevent.config.loop(default=True)
loop2 = gevent.config.loop(default=True)
self.assertTrue(loop1.default)
self.assertTrue(loop2.default)
# Destroy the first
loop1.destroy()
# It no longer claims to be the default
self.assertFalse(loop1.default)
# Destroy the second. This doesn't crash.
loop2.destroy()
if __name__ == '__main__':
unittest.main()
......@@ -125,3 +125,4 @@ test_timeout.py
test_asyncore.py
test___config.py
test__destroy_default_loop.py
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment