diff options
author | Petr Viktorin <pviktori@redhat.com> | 2015-05-26 13:25:12 +0200 |
---|---|---|
committer | Andrew Bartlett <abartlet@samba.org> | 2015-06-10 06:06:19 +0200 |
commit | 560576217f11b132f9c32de6e41c623dbc152137 (patch) | |
tree | 00ea621614defa31c8cbf2a1a3becb7388fa453b /lib | |
parent | 4399dc582fa06b04e1cec1d3aa59cd332e4b5ba2 (diff) | |
download | samba-560576217f11b132f9c32de6e41c623dbc152137.tar.gz |
pytalloc: Improve timer wrapper, and test it
Using Context.add_timer resulted in crashes due to missing type object
and bad reference handling.
Add a TeventTimer_Type struct, and introduce a clear ownership/lifetime model.
Add a "add_timer_offset" to allow adding timers from Python. (add_timer
requires passing struct timeval as a Python integer, which can't really
be done portably).
Add tests.
Signed-off-by: Petr Viktorin <pviktori@redhat.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jelmer Vernooij <jelmer@samba.org>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/tevent/bindings.py | 52 | ||||
-rw-r--r-- | lib/tevent/pytevent.c | 147 | ||||
-rwxr-xr-x | lib/tevent/wscript | 5 |
3 files changed, 185 insertions, 19 deletions
diff --git a/lib/tevent/bindings.py b/lib/tevent/bindings.py index 1060caf28df..55aafbb64a6 100644 --- a/lib/tevent/bindings.py +++ b/lib/tevent/bindings.py @@ -22,8 +22,10 @@ # License along with this library; if not, see <http://www.gnu.org/licenses/>. import signal +from unittest import TestCase, TestProgram +import gc + import _tevent -from unittest import TestCase class BackendListTests(TestCase): @@ -60,3 +62,51 @@ class ContextTests(TestCase): def test_add_signal(self): sig = self.ctx.add_signal(signal.SIGINT, 0, lambda callback: None) self.assertTrue(isinstance(sig, _tevent.Signal)) + + def test_timer(self): + """Test a timer is can be scheduled""" + collecting_list = [] + # time "0" has already passed, callback will be scheduled immediately + timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True)) + self.assertTrue(timer.active) + self.assertEqual(collecting_list, []) + self.ctx.loop_once() + self.assertFalse(timer.active) + self.assertEqual(collecting_list, [True]) + + def test_timer_deallocate_timer(self): + """Test timer is scheduled even if reference to it isn't held""" + collecting_list = [] + def callback(t): + collecting_list.append(True) + timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True)) + gc.collect() + self.assertEqual(collecting_list, []) + self.ctx.loop_once() + self.assertEqual(collecting_list, [True]) + + def test_timer_deallocate_context(self): + """Test timer is unscheduled when context is freed""" + collecting_list = [] + def callback(t): + collecting_list.append(True) + timer = self.ctx.add_timer(0, lambda t: collecting_list.append(True)) + self.assertTrue(timer.active) + del self.ctx + gc.collect() + self.assertEqual(collecting_list, []) + self.assertFalse(timer.active) + + def test_timer_offset(self): + """Test scheduling timer with an offset""" + collecting_list = [] + self.ctx.add_timer_offset(0.2, lambda t: collecting_list.append(2)) + self.ctx.add_timer_offset(0.1, lambda t: collecting_list.append(1)) + self.assertEqual(collecting_list, []) + self.ctx.loop_once() + self.assertEqual(collecting_list, [1]) + self.ctx.loop_once() + self.assertEqual(collecting_list, [1, 2]) + +if __name__ == '__main__': + TestProgram() diff --git a/lib/tevent/pytevent.c b/lib/tevent/pytevent.c index a495da5385d..a04d5657b0d 100644 --- a/lib/tevent/pytevent.c +++ b/lib/tevent/pytevent.c @@ -50,6 +50,7 @@ typedef struct { typedef struct { PyObject_HEAD struct tevent_timer *timer; + PyObject *callback; } TeventTimer_Object; typedef struct { @@ -372,38 +373,148 @@ static void py_timer_handler(struct tevent_context *ev, struct timeval current_time, void *private_data) { - PyObject *callback = private_data, *ret; - ret = PyObject_CallFunction(callback, "l", te); + TeventTimer_Object *self = private_data; + PyObject *ret; + + ret = PyObject_CallFunction(self->callback, "l", te); + if (ret == NULL) { + /* No Python stack to propagate exception to; just print traceback */ + PyErr_PrintEx(0); + } Py_XDECREF(ret); } -static PyObject *py_tevent_context_add_timer(TeventContext_Object *self, PyObject *args) +static void py_tevent_timer_dealloc(TeventTimer_Object *self) { - TeventTimer_Object *ret; - struct timeval next_event; - struct tevent_timer *timer; - PyObject *handler; - if (!PyArg_ParseTuple(args, "lO", &next_event, &handler)) - return NULL; - - timer = tevent_add_timer(self->ev, NULL, next_event, py_timer_handler, - handler); - if (timer == NULL) { - PyErr_SetNone(PyExc_RuntimeError); - return NULL; + if (self->timer) { + talloc_free(self->timer); } + Py_DECREF(self->callback); + PyObject_Del(self); +} + +static int py_tevent_timer_traverse(TeventTimer_Object *self, visitproc visit, void *arg) +{ + Py_VISIT(self->callback); + return 0; +} + +static PyObject* py_tevent_timer_get_active(TeventTimer_Object *self) { + return PyBool_FromLong(self->timer != NULL); +} + +struct PyGetSetDef py_tevent_timer_getset[] = { + { + .name = "active", + .get = (getter)py_tevent_timer_get_active, + .doc = "true if the timer is scheduled to run", + }, + {NULL}, +}; + +static PyTypeObject TeventTimer_Type = { + .tp_name = "tevent.Timer", + .tp_basicsize = sizeof(TeventTimer_Object), + .tp_dealloc = (destructor)py_tevent_timer_dealloc, + .tp_traverse = (traverseproc)py_tevent_timer_traverse, + .tp_getset = py_tevent_timer_getset, + .tp_flags = Py_TPFLAGS_DEFAULT, +}; + +static int timer_destructor(void* ptr) +{ + TeventTimer_Object *obj = *(TeventTimer_Object **)ptr; + obj->timer = NULL; + Py_DECREF(obj); + return 0; +} + +static PyObject *py_tevent_context_add_timer_internal(TeventContext_Object *self, + struct timeval next_event, + PyObject *callback) +{ + /* Ownership notes: + * + * There are 5 pieces in play; two tevent contexts and 3 Python objects: + * - The tevent timer + * - The tevent context + * - The Python context -- "self" + * - The Python timer (TeventTimer_Object) -- "ret" + * - The Python callback function -- "callback" + * + * We only use the Python context for getting the tevent context, + * afterwards it can be destroyed. + * + * The tevent context owns the tevent timer. + * + * The tevent timer holds a reference to the Python timer, so the Python + * timer must always outlive the tevent timer. + * The Python timer has a pointer to the tevent timer; a destructor is + * used to set this to NULL when the tevent timer is deallocated. + * + * The tevent timer can be deallocated in these cases: + * 1) when the context is destroyed + * 2) after the event fires + * Posssibly, API might be added to cancel (free the tevent timer). + * + * The Python timer holds a reference to the callback. + */ + TeventTimer_Object *ret; + TeventTimer_Object **tmp_context; ret = PyObject_New(TeventTimer_Object, &TeventTimer_Type); if (ret == NULL) { PyErr_NoMemory(); - talloc_free(timer); return NULL; } - ret->timer = timer; + Py_INCREF(callback); + ret->callback = callback; + ret->timer = tevent_add_timer(self->ev, NULL, next_event, py_timer_handler, + ret); + if (ret->timer == NULL) { + Py_DECREF(ret); + PyErr_SetString(PyExc_RuntimeError, "Could not initialize timer"); + return NULL; + } + tmp_context = talloc(ret->timer, TeventTimer_Object*); + if (tmp_context == NULL) { + talloc_free(ret->timer); + Py_DECREF(ret); + PyErr_SetString(PyExc_RuntimeError, "Could not initialize timer"); + return NULL; + } + Py_INCREF(ret); + *tmp_context = ret; + talloc_set_destructor(tmp_context, timer_destructor); return (PyObject *)ret; } +static PyObject *py_tevent_context_add_timer(TeventContext_Object *self, PyObject *args) +{ + struct timeval next_event; + PyObject *callback; + if (!PyArg_ParseTuple(args, "lO", &next_event, &callback)) + return NULL; + + return py_tevent_context_add_timer_internal(self, next_event, callback); +} + +static PyObject *py_tevent_context_add_timer_offset(TeventContext_Object *self, PyObject *args) +{ + struct timeval next_event; + double offset; + int seconds; + PyObject *callback; + if (!PyArg_ParseTuple(args, "dO", &offset, &callback)) + return NULL; + + seconds = offset; + offset -= seconds; + next_event = tevent_timeval_current_ofs(seconds, (int)(offset*1000000)); + return py_tevent_context_add_timer_internal(self, next_event, callback); +} + static void py_fd_handler(struct tevent_context *ev, struct tevent_fd *fde, uint16_t flags, @@ -479,6 +590,8 @@ static PyMethodDef py_tevent_context_methods[] = { METH_VARARGS, "S.add_signal(signum, sa_flags, handler) -> signal" }, { "add_timer", (PyCFunction)py_tevent_context_add_timer, METH_VARARGS, "S.add_timer(next_event, handler) -> timer" }, + { "add_timer_offset", (PyCFunction)py_tevent_context_add_timer_offset, + METH_VARARGS, "S.add_timer(offset_seconds, handler) -> timer" }, { "add_fd", (PyCFunction)py_tevent_context_add_fd, METH_VARARGS, "S.add_fd(fd, flags, handler) -> fd" }, #ifdef TEVENT_DEPRECATED diff --git a/lib/tevent/wscript b/lib/tevent/wscript index 026cd9a1c1b..d0f1ac18366 100755 --- a/lib/tevent/wscript +++ b/lib/tevent/wscript @@ -13,7 +13,7 @@ while not os.path.exists(srcdir+'/buildtools') and len(srcdir.split('/')) < 5: srcdir = srcdir + '/..' sys.path.insert(0, srcdir + '/buildtools/wafsamba') -import wafsamba, samba_dist, Options, Logs +import wafsamba, samba_dist, samba_utils, Options, Logs samba_dist.DIST_DIRS('lib/tevent:. lib/replace:lib/replace lib/talloc:lib/talloc buildtools:buildtools third_party/waf:third_party/waf') @@ -130,6 +130,9 @@ def test(ctx): '''test tevent''' print("The tevent testsuite is part of smbtorture in samba4") + pyret = samba_utils.RUN_PYTHON_TESTS(['bindings.py']) + sys.exit(pyret) + def dist(): '''makes a tarball for distribution''' |