kore

commit bb2f0d8b52630a8b8b640edee18e83dd7b8946d3
parent 8235759bca283efe1e787813f18d709b8367e977
Author: Joris Vink <joris@coders.se>
Date:   Thu,  9 Jul 2020 20:22:18 +0200

Python: improve kore.lock when handling cancelled coroutines.

If a coroutine is killed from another coroutine and the killed coroutine
was waiting on a kore.lock() object, it would have been incorrectly
woken up again once said lock was released.

This would cause a Python exception that a generator was already
running and a crash due to the pool element already being freed.

Track the active locking operation per coroutine so we can remove
the coroutine if it is killed, fixing the problem.

Diffstat:
include/kore/python_methods.h
 | 
1
+
src/python.c
 | 
25
++++++++++++++++++++++++-

2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/kore/python_methods.h b/include/kore/python_methods.h
@@ -24,6 +24,7 @@ struct python_coro {
 	PyObject			*obj;
 	char				*name;
 	PyObject			*result;
+	struct pylock_op		*lockop;
 	struct pysocket_op		*sockop;
 	struct pygather_op		*gatherop;
 	struct http_request		*request;
diff --git a/src/python.c b/src/python.c
@@ -410,6 +410,13 @@ kore_python_coro_delete(void *obj)
 	Py_DECREF(coro->obj);
 	coro_running = NULL;
 
+	if (coro->lockop != NULL) {
+		coro->lockop->active = 0;
+		TAILQ_REMOVE(&coro->lockop->lock->ops, coro->lockop, list);
+		Py_DECREF((PyObject *)coro->lockop);
+		coro->lockop = NULL;
+	}
+
 	if (coro->state == CORO_STATE_RUNNABLE)
 		TAILQ_REMOVE(&coro_runnable, coro, list);
 	else
@@ -947,6 +954,7 @@ python_coro_create(PyObject *obj, struct http_request *req)
 	coro->name = NULL;
 	coro->result = NULL;
 	coro->sockop = NULL;
+	coro->lockop = NULL;
 	coro->gatherop = NULL;
 	coro->exception = NULL;
 	coro->exception_msg = NULL;
@@ -3575,6 +3583,7 @@ pylock_dealloc(struct pylock *lock)
 	while ((op = TAILQ_FIRST(&lock->ops)) != NULL) {
 		TAILQ_REMOVE(&lock->ops, op, list);
 		op->active = 0;
+		op->coro->lockop = NULL;
 		Py_DECREF((PyObject *)op);
 	}
 
@@ -3615,6 +3624,9 @@ pylock_aenter(struct pylock *lock, PyObject *args)
 {
 	struct pylock_op	*op;
 
+	if (coro_running->lockop != NULL)
+		fatal("%s: lockop not NULL for %u", __func__, coro_running->id);
+
 	if (lock->owner != NULL && lock->owner->id == coro_running->id) {
 		PyErr_SetString(PyExc_RuntimeError, "recursive lock detected");
 		return (NULL);
@@ -3628,6 +3640,8 @@ pylock_aenter(struct pylock *lock, PyObject *args)
 	op->locking = 1;
 	op->coro = coro_running;
 
+	coro_running->lockop = op;
+
 	Py_INCREF((PyObject *)op);
 	Py_INCREF((PyObject *)lock);
 
@@ -3641,6 +3655,9 @@ pylock_aexit(struct pylock *lock, PyObject *args)
 {
 	struct pylock_op	*op;
 
+	if (coro_running->lockop != NULL)
+		fatal("%s: lockop not NULL for %u", __func__, coro_running->id);
+
 	if (lock->owner == NULL || lock->owner->id != coro_running->id) {
 		PyErr_SetString(PyExc_RuntimeError, "invalid lock owner");
 		return (NULL);
@@ -3654,6 +3671,8 @@ pylock_aexit(struct pylock *lock, PyObject *args)
 	op->locking = 0;
 	op->coro = coro_running;
 
+	coro_running->lockop = op;
+
 	Py_INCREF((PyObject *)op);
 	Py_INCREF((PyObject *)lock);
 
@@ -3674,7 +3693,8 @@ pylock_do_release(struct pylock *lock)
 			continue;
 
 		op->active = 0;
-		TAILQ_REMOVE(&op->lock->ops, op, list);
+		op->coro->lockop = NULL;
+		TAILQ_REMOVE(&lock->ops, op, list);
 
 		if (op->coro->request != NULL)
 			http_request_wakeup(op->coro->request);
@@ -3694,6 +3714,8 @@ pylock_op_dealloc(struct pylock_op *op)
 		op->active = 0;
 	}
 
+	op->coro->lockop = NULL;
+
 	Py_DECREF((PyObject *)op->lock);
 	PyObject_Del((PyObject *)op);
 }
@@ -3741,6 +3763,7 @@ pylock_op_iternext(struct pylock_op *op)
 
 	if (op->active) {
 		op->active = 0;
+		op->coro->lockop = NULL;
 		TAILQ_REMOVE(&op->lock->ops, op, list);
 		Py_DECREF((PyObject *)op);
 	}