Skip to content

Commit 6b429a5

Browse files
committed
Stackless issue python#220: fix hard switching error handling
Add a test function to provoke switching errors, add test cases and fix the error handling. (cherry picked from commit 8ae5fe5)
1 parent 7a2fba7 commit 6b429a5

8 files changed

Lines changed: 156 additions & 17 deletions

File tree

‎Stackless/changelog.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ What's New in Stackless 3.X.X?
99

1010
*Release date: 20XX-XX-XX*
1111

12+
- https://github.com/stackless-dev/stackless/issues/220
13+
Improve the error handling in case of failed stack transfers / hard tasklet
14+
switches. Call Py_FatalError, if a clean recovery is impossible.
15+
1216
- https://github.com/stackless-dev/stackless/issues/217
1317
C-API documentation update: update the names of watchdog flags to match the
1418
implementation.

‎Stackless/core/stacklesseval.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ eval_frame_callback(PyFrameObject *f, int exc, PyObject *retval)
892892
SLP_FRAME_EXECFUNC_DECREF(cf);
893893
slp_transfer_return(cst);
894894
/* should never come here */
895-
assert(0);
895+
Py_FatalError("Return from stack spilling failed.");
896896
fatal:
897897
SLP_STORE_NEXT_FRAME(ts, cf->f_back);
898898
return NULL;

‎Stackless/module/scheduling.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,7 @@ kill_wrap_bad_guy(PyTaskletObject *prev, PyTaskletObject *bad_guy)
534534
if (prev->next == NULL)
535535
slp_current_insert(prev);
536536
SLP_STORE_NEXT_FRAME(ts, prev->f.frame);
537+
Py_CLEAR(prev->f.frame);
537538
ts->st.current = prev;
538539
if (newval != NULL) {
539540
/* merge bad guy into exception */
@@ -646,8 +647,11 @@ jump_soft_to_hard(PyFrameObject *f, int exc, PyObject *retval)
646647
Py_DECREF(retval); /* consume ref according to protocol */
647648
SLP_FRAME_EXECFUNC_DECREF(f);
648649
slp_transfer_return(ts->st.current->cstate);
649-
/* we either have an error or don't come back, so: */
650-
assert(0);
650+
/* We either have an error or don't come back, so bail out.
651+
* There is no way to recover, because we don't know the previous
652+
* tasklet.
653+
*/
654+
Py_FatalError("Soft-to-hard tasklet switch failed.");
651655
return NULL;
652656
}
653657

@@ -1028,7 +1032,9 @@ slp_schedule_task(PyObject **result, PyTaskletObject *prev, PyTaskletObject *nex
10281032

10291033
fail = slp_schedule_task_prepared(ts, result, prev, next, stackless, did_switch);
10301034
if (fail && inserted) {
1031-
slp_current_uninsert(next);
1035+
/* in case of an error, it is unknown, if the tasklet is still scheduled */
1036+
if (next->next)
1037+
slp_current_uninsert(next);
10321038
if (u_chan)
10331039
slp_channel_insert(u_chan, next, u_dir, u_next);
10341040
else
@@ -1266,7 +1272,7 @@ slp_schedule_task_prepared(PyThreadState *ts, PyObject **result, PyTaskletObject
12661272
}
12671273
else {
12681274
PyFrameObject *f = SLP_CLAIM_NEXT_FRAME(ts);
1269-
Py_XDECREF(f);
1275+
Py_XSETREF(next->f.frame, f); /* revert the Py_CLEAR(next->f.frame) */
12701276
kill_wrap_bad_guy(prev, next);
12711277
return -1;
12721278
}
@@ -1477,8 +1483,7 @@ slp_tasklet_end(PyObject *retval)
14771483
Py_DECREF(retval);
14781484
SLP_STORE_NEXT_FRAME(ts, NULL);
14791485
slp_transfer_return(ts->st.initial_stub); /* does not return */
1480-
assert(0);
1481-
return NULL;
1486+
Py_FatalError("Failed to restore the initial C-stack.");
14821487
}
14831488
}
14841489

‎Stackless/module/stacklessmodule.c‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,9 @@ PyStackless_RunWatchdogEx(long timeout, int flags)
532532
/* we failed to switch */
533533
PyTaskletObject *undo = pop_watchdog(ts);
534534
assert(undo == old_current);
535-
slp_current_unremove(undo);
535+
/* In case of an error, we don't know the state */
536+
if (undo->next == NULL)
537+
slp_current_unremove(undo);
536538
return NULL;
537539
}
538540

@@ -944,6 +946,7 @@ PyDoc_STRVAR(test_cframe_nr__doc__,
944946
"test_cframe_nr(switches) -- a builtin testing function that does nothing\n\
945947
but soft tasklet switching. The function will call PyStackless_Schedule_nr() for switches\n\
946948
times and then finish.\n\
949+
All remaining arguments are intentionally undocumented. Don't use them!\n\
947950
Usage: Cf. test_cframe().");
948951

949952
static
@@ -976,14 +979,23 @@ static
976979
PyObject *
977980
test_cframe_nr(PyObject *self, PyObject *args, PyObject *kwds)
978981
{
979-
static char *argnames[] = {"switches", NULL};
982+
static char *argnames[] = {"switches", "cstate_add_addr", NULL};
980983
PyThreadState *ts = PyThreadState_GET();
981984
PyCFrameObject *cf;
982985
long switches;
986+
PyObject *cstack = NULL;
983987

984-
if (!PyArg_ParseTupleAndKeywords(args, kwds, "l:test_cframe_nr",
985-
argnames, &switches))
988+
if (!PyArg_ParseTupleAndKeywords(args, kwds, "l|O!:test_cframe_nr",
989+
argnames, &switches, &PyCStack_Type, &cstack))
986990
return NULL;
991+
if (cstack) {
992+
/* Manipulate the startaddr of a cstack to make it (in)valid.
993+
* This can be used to provoke switching errors. Without such a functionality,
994+
* it is not possible to test the error handling code for switching errors.
995+
*/
996+
((PyCStackObject*)cstack)->startaddr += switches;
997+
Py_RETURN_NONE;
998+
}
987999
cf = slp_cframe_new(test_cframe_nr_loop, 1);
9881000
if (cf == NULL)
9891001
return NULL;

‎Stackless/module/taskletobject.c‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ slp_current_remove_tasklet(PyTaskletObject *task)
109109
PyThreadState *ts = task->cstate->tstate;
110110
PyTaskletObject **chain = &ts->st.current, *ret, *hold;
111111

112+
/* Make sure the tasklet is scheduled.
113+
*/
114+
assert(task->next != NULL);
115+
assert(task->prev != NULL);
112116
assert(ts != NULL);
113117
--ts->st.runcount;
114118
assert(ts->st.runcount >= 0);
@@ -891,11 +895,13 @@ impl_tasklet_run_remove(PyTaskletObject *task, int remove)
891895
if (removed) {
892896
assert(ts->st.del_post_switch == (PyObject *)prev);
893897
ts->st.del_post_switch = NULL;
894-
slp_current_unremove(prev);
898+
if (prev->next == NULL) /* in case of an error, the state is unknown */
899+
slp_current_unremove(prev);
895900
}
896901
if (inserted) {
897-
/* we must undo the insertion that we did */
898-
slp_current_uninsert(task);
902+
/* we must undo the insertion that we did, but we don't know the state */
903+
if (task->next != NULL)
904+
slp_current_uninsert(task);
899905
Py_DECREF(task);
900906
}
901907
} else if (!switched) {

‎Stackless/pickling/safe_pickle.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pickle_callback(PyFrameObject *f, int exc, PyObject *retval)
4646
SLP_FRAME_EXECFUNC_DECREF(f);
4747
slp_transfer_return(cst);
4848
/* never come here */
49-
assert(0);
49+
Py_FatalError("Return from pickle stack spilling failed.");
5050
return NULL;
5151
}
5252

‎Stackless/unittests/support.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
try:
4747
import threading
4848
withThreads = True
49-
except:
49+
except Exception:
5050
withThreads = False
5151

5252

@@ -696,6 +696,7 @@ def _addSkip(self, result, reason):
696696
self._ran_AsTaskletTestCase_setUp = True
697697
super(StacklessTestCase, self)._addSkip(result, reason)
698698

699+
699700
_tc = StacklessTestCase(methodName='run')
700701
try:
701702
_tc.setUp()
@@ -741,7 +742,7 @@ def run(self, result=None):
741742
def helper():
742743
try:
743744
c.send(super(AsTaskletTestCase, self).run(result))
744-
except:
745+
except: # @IgnorePep8
745746
c.send_throw(*sys.exc_info())
746747
stackless.tasklet(helper)()
747748
result = c.receive()
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
from __future__ import absolute_import, division, print_function
2+
3+
import unittest
4+
import sys
5+
import stackless
6+
import contextlib
7+
8+
from support import test_main # @UnusedImport
9+
from support import StacklessTestCase, testcase_leaks_references
10+
11+
12+
class TestStackTransferFailures(StacklessTestCase):
13+
"""Test the error handling of failures in slp_transfer()
14+
15+
In theory, slp_transfer can't fail, but in reality there bugs. Unfortunately
16+
it is usually not possible to recover from slp_transfer failures. But we try to avoid
17+
undefined behavior.
18+
19+
The test coverage is incomplete, patches are welcome. Especially failed soft-to-hard switches
20+
result in Py_FatalError("Soft-to-hard tasklet switch failed.").
21+
"""
22+
def setUp(self):
23+
StacklessTestCase.setUp(self)
24+
25+
@contextlib.contextmanager
26+
def corrupted_cstack_startaddr(self, tasklet):
27+
"""Increment the cstack startaddr of this tasklet.
28+
29+
Hard-switching to this C-stack will fail. This context is used
30+
by error handling tests
31+
"""
32+
self.assertTrue(tasklet.alive)
33+
self.assertEqual(tasklet.thread_id, stackless.current.thread_id)
34+
self.assertFalse(tasklet.restorable)
35+
self.assertFalse(tasklet.is_current)
36+
cstate = tasklet.cstate
37+
self.assertNotEqual(cstate.startaddr, 0) # not invalidated
38+
self.assertIs(cstate.task, tasklet)
39+
test_cframe_nr = stackless.test_cframe_nr
40+
# Otherwise undocumented and only for this test.
41+
# Add 1 to the startaddr of cstate
42+
test_cframe_nr(1, cstate_add_addr=cstate)
43+
try:
44+
yield
45+
finally:
46+
# Restore the pprevious value
47+
test_cframe_nr(-1, cstate_add_addr=cstate)
48+
49+
def __task(self):
50+
stackless.schedule_remove(None)
51+
self.tasklet_done = True
52+
53+
def prepare_tasklet(self, task=None):
54+
if task is None:
55+
task = self.__task
56+
57+
self.tasklet_done = False
58+
t = stackless.tasklet(self.__task)()
59+
sw = stackless.enable_softswitch(False)
60+
try:
61+
t.run()
62+
finally:
63+
stackless.enable_softswitch(sw)
64+
return t
65+
66+
def end(self, t):
67+
# if t.alive:
68+
t.run()
69+
self.assertTrue(self.tasklet_done)
70+
71+
@testcase_leaks_references("unknown")
72+
def test_scheduler(self):
73+
t = self.prepare_tasklet()
74+
with self.corrupted_cstack_startaddr(t):
75+
t.insert()
76+
self.assertRaisesRegex(SystemError, 'bad stack reference in transfer', stackless.run)
77+
self.end(t)
78+
test_scheduler.enable_softswitch = False # soft switching causes soft-to-hard
79+
80+
@testcase_leaks_references("unknown")
81+
def test_run(self):
82+
t = self.prepare_tasklet()
83+
with self.corrupted_cstack_startaddr(t):
84+
self.assertRaisesRegex(SystemError, 'bad stack reference in transfer', t.run)
85+
self.end(t)
86+
test_run.enable_softswitch = False # soft switching causes soft-to-hard
87+
88+
@testcase_leaks_references("unknown")
89+
def test_switch(self):
90+
t = self.prepare_tasklet()
91+
with self.corrupted_cstack_startaddr(t):
92+
self.assertRaisesRegex(SystemError, 'bad stack reference in transfer', t.switch)
93+
self.end(t)
94+
test_switch.enable_softswitch = False # soft switching causes soft-to-hard
95+
96+
@testcase_leaks_references("unknown")
97+
def test_kill(self):
98+
t = self.prepare_tasklet()
99+
with self.corrupted_cstack_startaddr(t):
100+
self.assertRaisesRegex(SystemError, 'bad stack reference in transfer', t.kill)
101+
self.end(t)
102+
test_kill.enable_softswitch = False # soft switching causes soft-to-hard
103+
104+
105+
#///////////////////////////////////////////////////////////////////////////////
106+
107+
if __name__ == '__main__':
108+
if not sys.argv[1:]:
109+
sys.argv.append('-v')
110+
111+
unittest.main()

0 commit comments

Comments
 (0)