Visitar URL original
Stackless issue #220: fix hard switching error handling · akruis/cpython@908439c · GitHub
Skip to content

Commit 908439c

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 669e77d commit 908439c

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
@@ -10,6 +10,10 @@ What's New in Stackless 2.7.XX?
1010

1111
*Release date: XXXX-XX-XX*
1212

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

‎Stackless/core/stacklesseval.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ eval_frame_callback(PyFrameObject *f, int exc, PyObject *retval)
911911
SLP_FRAME_EXECFUNC_DECREF(cf);
912912
slp_transfer_return(cst);
913913
/* should never come here */
914-
assert(0);
914+
Py_FatalError("Return from stack spilling failed.");
915915
fatal:
916916
SLP_STORE_NEXT_FRAME(ts, cf->f_back);
917917
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 */
@@ -651,8 +652,11 @@ jump_soft_to_hard(PyFrameObject *f, int exc, PyObject *retval)
651652
Py_DECREF(retval); /* consume ref according to protocol */
652653
SLP_FRAME_EXECFUNC_DECREF(f);
653654
slp_transfer_return(ts->st.current->cstate);
654-
/* we either have an error or don't come back, so: */
655-
assert(0);
655+
/* We either have an error or don't come back, so bail out.
656+
* There is no way to recover, because we don't know the previous
657+
* tasklet.
658+
*/
659+
Py_FatalError("Soft-to-hard tasklet switch failed.");
656660
return NULL;
657661
}
658662

@@ -1033,7 +1037,9 @@ slp_schedule_task(PyObject **result, PyTaskletObject *prev, PyTaskletObject *nex
10331037

10341038
fail = slp_schedule_task_prepared(ts, result, prev, next, stackless, did_switch);
10351039
if (fail && inserted) {
1036-
slp_current_uninsert(next);
1040+
/* in case of an error, it is unknown, if the tasklet is still scheduled */
1041+
if (next->next)
1042+
slp_current_uninsert(next);
10371043
if (u_chan)
10381044
slp_channel_insert(u_chan, next, u_dir, u_next);
10391045
else
@@ -1275,7 +1281,7 @@ slp_schedule_task_prepared(PyThreadState *ts, PyObject **result, PyTaskletObject
12751281
}
12761282
else {
12771283
PyFrameObject *f = SLP_CLAIM_NEXT_FRAME(ts);
1278-
Py_XDECREF(f);
1284+
Py_XSETREF(next->f.frame, f); /* revert the Py_CLEAR(next->f.frame) */
12791285
kill_wrap_bad_guy(prev, next);
12801286
return -1;
12811287
}
@@ -1486,8 +1492,7 @@ slp_tasklet_end(PyObject *retval)
14861492
Py_DECREF(retval);
14871493
SLP_STORE_NEXT_FRAME(ts, NULL);
14881494
slp_transfer_return(ts->st.initial_stub); /* does not return */
1489-
assert(0);
1490-
return NULL;
1495+
Py_FatalError("Failed to restore the initial C-stack.");
14911496
}
14921497
}
14931498

‎Stackless/module/stacklessmodule.c‎

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

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

948951
static
@@ -975,14 +978,23 @@ static
975978
PyObject *
976979
test_cframe_nr(PyObject *self, PyObject *args, PyObject *kwds)
977980
{
978-
static char *argnames[] = {"switches", NULL};
981+
static char *argnames[] = {"switches", "cstate_add_addr", NULL};
979982
PyThreadState *ts = PyThreadState_GET();
980983
PyCFrameObject *cf;
981984
long switches;
985+
PyObject *cstack = NULL;
982986

983-
if (!PyArg_ParseTupleAndKeywords(args, kwds, "l:test_cframe_nr",
984-
argnames, &switches))
987+
if (!PyArg_ParseTupleAndKeywords(args, kwds, "l|O!:test_cframe_nr",
988+
argnames, &switches, &PyCStack_Type, &cstack))
985989
return NULL;
990+
if (cstack) {
991+
/* Manipulate the startaddr of a cstack to make it (in)valid.
992+
* This can be used to provoke switching errors. Without such a functionality,
993+
* it is not possible to test the error handling code for switching errors.
994+
*/
995+
((PyCStackObject*)cstack)->startaddr += switches;
996+
Py_RETURN_NONE;
997+
}
986998
cf = slp_cframe_new(test_cframe_nr_loop, 1);
987999
if (cf == NULL)
9881000
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);
@@ -884,11 +888,13 @@ impl_tasklet_run_remove(PyTaskletObject *task, int remove)
884888
if (removed) {
885889
assert(ts->st.del_post_switch == (PyObject *)prev);
886890
ts->st.del_post_switch = NULL;
887-
slp_current_unremove(prev);
891+
if (prev->next == NULL) /* in case of an error, the state is unknown */
892+
slp_current_unremove(prev);
888893
}
889894
if (inserted) {
890-
/* we must undo the insertion that we did */
891-
slp_current_uninsert(task);
895+
/* we must undo the insertion that we did, but we don't know the state */
896+
if (task->next != NULL)
897+
slp_current_uninsert(task);
892898
Py_DECREF(task);
893899
}
894900
} 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
@@ -45,7 +45,7 @@
4545
try:
4646
import threading
4747
withThreads = True
48-
except:
48+
except Exception:
4949
withThreads = False
5050

5151

@@ -681,6 +681,7 @@ def _addSkip(self, result, reason):
681681
self._ran_AsTaskletTestCase_setUp = True
682682
super(StacklessTestCase, self)._addSkip(result, reason)
683683

684+
684685
_tc = StacklessTestCase(methodName='run')
685686
try:
686687
_tc.setUp()
@@ -726,7 +727,7 @@ def run(self, result=None):
726727
def helper():
727728
try:
728729
c.send(super(AsTaskletTestCase, self).run(result))
729-
except:
730+
except: # @IgnorePep8
730731
c.send_throw(*sys.exc_info())
731732
stackless.tasklet(helper)()
732733
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)