python / cpython

The Python programming language
https://www.python.org
Other
62.71k stars 30.06k forks source link

test_cpickle overflows stack on MacOS9 #38027

Closed jackjansen closed 21 years ago

jackjansen commented 21 years ago
BPO 690622
Nosy @gvanrossum, @tim-one, @jackjansen

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = 'https://github.com/jackjansen' closed_at = created_at = labels = [] title = 'test_cpickle overflows stack on MacOS9' updated_at = user = 'https://github.com/jackjansen' ``` bugs.python.org fields: ```python activity = actor = 'jackjansen' assignee = 'jackjansen' closed = True closed_date = None closer = None components = ['None'] creation = creator = 'jackjansen' dependencies = [] files = [] hgrepos = [] issue_num = 690622 keywords = [] message_count = 6.0 messages = ['14765', '14766', '14767', '14768', '14769', '14770'] nosy_count = 3.0 nosy_names = ['gvanrossum', 'tim.peters', 'jackjansen'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue690622' versions = [] ```

jackjansen commented 21 years ago

Test_cpickle overflows the stack and hangs the interpreter on MacOS9. This happens in test_nonrecursive_deep(), in the dumps() call.

I think there's a PyOS_CheckStack() call missing somewhere in cPickle.

Also, it's probably a good idea to lower the recursion to 50 (the test passes as long as the recusrion is less than 65 deep), at least on MacOS9.

tim-one commented 21 years ago

Logged In: YES user_id=31435

There are no PyOS_CheckStack() calls in cPickle, and I'm not sure anyone really wants to bother adding them (cPickle is meant to be high-performance, not safe \<wink>).

That test is consuming more stack frames than it used to, because each level of list pickling now bites another stack frame to call the new "batch list elements" routine.

Reducing to 50 doesn't fly because cPickle's internal PY_CPICKLE_FAST_LIMIT #define happens to be 50 now, and the purpose of test_nonrecursive_deep is to be sure that non-cyclic containers get pickled OK in a "fast" pickler even if they're nested as deeply as PY_CPICKLE_FAST_LIMIT.

Assigning to Guido for further cogitation on his day off \<wink>.

gvanrossum commented 21 years ago

Logged In: YES user_id=6380

I agree that I'd rather not see CheckStack calls added to cPickle. But using 60 instead of 100 should work for both of you, right?

tim-one commented 21 years ago

Logged In: YES user_id=31435

Right, AFAICT 60 is enough to test what this is trying to test (although I'm not the "fast pickler" expert -- but I think it's clear enough). Jack, please confirm that 60 is OK for you?

tim-one commented 21 years ago

Logged In: YES user_id=31435

I reduced the nesting depth to 60, in Lib/test/test_cpickle.py, rev 1.14. Jack, please close this bug as Fixed if your problem goes away now.

jackjansen commented 21 years ago

Logged In: YES user_id=45365

It passes now. Closing.