Crash after unpickling hard switched tasklet

[ghost]

Originally reported by: Anselm Kruis (Bitbucket: akruis, GitHub: akruis)

---

Hi,

a few weeks ago I observed crashes during the development of pyheapdump. Now I had some time to track them down and to write a small example.

The attached script first pickles a single frame and unpickles the frame. Then it accesses frame.f_locals. Python crashes in function map_to_dict() from frameobject.c. This function is invoked from PyFrame_FastToLocals().

Obviously we have two bugs:

1. Upon unpickling python fails to set up frame->f_localsplus correctly (== as expected by PyFrame_FastToLocals) in function frame_setstate from prickelpit.c. The localsplus array must start with frame->f_code->co_nlocals local variables, followed by len(frame->f_code->co_cellvars) + len(frame->f_code->co_freevars) cell objects.

2. Upon pickling python creates an inconsistent object state for the frame in function frameobject_reduce from prickelpit.c. If the frame has no stacktop, python omits the localsplus array from the pickle. This would be correct, if the only valid use case for pickling frames was pickling, unpickling and resuming of tasklets. But is is also possible to pickle/unpickle a tasklet with the sole intent to inspect it using a debugger.

Plan

I'll add test cases for the two bugs and prepare pull requests with fixes.

Here is the code:

#!python
import pickle
import stackless


def pickle_current_frame():
    result = []

    def func(current):
        result.append(pickle.dumps(current.frame, -1))
    stackless.tasklet().bind(func, (stackless.current,)).run()
    return result[0]


soft = stackless.enable_softswitch(False)  # no crash, if soft switching
try:
    p = pickle_current_frame()
finally:
    stackless.enable_softswitch(soft)
frame = pickle.loads(p)
frame.f_locals  # this line crashes Stackless
print("No Crash, OK")

f_locals_crash.py.zip

---

• Bitbucket: https://bitbucket.org/stackless-dev/stackless/issue/61

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

I just created pull request #10 to fix bug 1 (crash after unpickling).

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

I just added pull request #11 to fix the second bug.

And I discovered '\r\n' line endings in the new test script Stackless/test/unpickle_crash_issue61.py added by pull request #10, commit fb46ecf79d49. We must not merge this pull request. I'll recreate it.

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

Created pull request #12 as a replacement for pull request #10.

[ghost]

Original comment by Christian Tismer (Bitbucket: ctismer, GitHub: ctismer):

---

I agree this is a bug.
The use-case of just doing introspection was uncommon and not intended
to be used before you did, but it is correct that this is not correct. ;-)

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

OK, I merged the pull requests into 2.7-slp, grafted the changes into 3.2-slp and merged them into 3.3-slp, 3.4-slp and default-slp.

@ctismer I don't have a 2.8-slp sandbox right now, so please could you merge the changes into 2.8-slp.

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

It's already fixed.

[ghost]

Original comment by Christian Tismer (Bitbucket: ctismer, GitHub: ctismer):

---

Oupss - I will try to merge.
Not trivial, I need to take tiny steps, because this is partially
3.4 stuff which now needs to be separated, again.

[ghost]

Original comment by Anselm Kruis (Bitbucket: akruis, GitHub: akruis):

---

Should be fairly simple:

1. create a new sandbox in case something breaks
2. make a null merge of the last changeset of 2.7-slp before the Crash after unpickling hard switched tasklet #61 fixes. changeset id is 67088aa
3. merge 2.7-slp. you get the fixes for this bug only
4. test everything is ok

#!/bin/sh
hg pull
hg checkout 2.8-slp

# now the null merge
hg merge 67088aa
hg revert -ar 2.8-slp
hg resolve -am
hg commit -m 'issue #61: null merge with 2.7-slp'

# now the merge of the bug fix
hg merge 2.7-slp
hg commit -m 'merge with 2.7-slp'

See https://docs.python.org/devguide/faq.html#id48 for further information.

[ghost]

Original comment by Christian Tismer (Bitbucket: ctismer, GitHub: ctismer):

---

Ah, thanks! I tried a merge using SourceTree, but reverted that.

What I did not understand in the first place was what to do about the changes of
8ac180d49f19 "Remove all non-Stackless-patch changes bringing the branch to v2.7.6 level. "
Actually that was to be ignored and needs to be done for a 2.8 release, separately.

Actually, I think it is time to publish 2.8 now, if we still want to do that.

Is it true that the repo is already gone public, or was that by chance in the sprint?

[ghost]

Original comment by Christian Tismer (Bitbucket: ctismer, GitHub: ctismer):

---

Fixed a bit and verified by

#!python

make test
make teststackless

on OS/X. Is that sufficient?