...

...

...

...

"CT" == Christian Tismer writes:

CT> I think it is fine that it crashed. There are obviously CT> extension modules left where the interpreter lock rule is CT> violated. The builtin Python code has been checked, there are CT> most probably no holes, including tkinter. Or, I made a mistake CT> in this little code: I think have misunderstood at least one of Mark's bug report and your response. Does the problem Mark reported rely on extension code? I thought the bug was triggered by running pure Python code. If that is the case, then it can never be fine that it crashed. If the problem relies on extension code, then there ought to be a way to write the extension so that it doesn't cause a crash. Jeremy PS Mark: Is the transformer.py you attached different from the one in the nondist/src/Compiler tree? It looks like the only differences are with the whitespace.

If it is so, then there is in fact a problem left in the Kernel. Mark, did you use an extension?

I tried to explain this in private email: This is pure Python code. The parser module is the only extension being used. The crash _always_ occurs as a frame object is being de-allocated, and _always_ happens as a builtin list object (a local variable) is de-alloced by the frame. Always the same line of Python code, always the same line of C code, always the exact same failure. Mark.

On Tue, 11 Apr 2000, Mark Hammond wrote:

[Sorry - missed this bit]

...

PS Mark: Is the transformer.py you attached different from the one in the nondist/src/Compiler tree? It looks like the only differences are with the whitespace.

The attached version is simply the "release" P2C transformer.py with .append args fixed. I imagine it is very close to the CVS version (and indeed I know for a fact that the CVS version also crashes).

My initial testing showed the CVS compiler did _not_ trigger this bug (even though code that uses an identical transformer.py does), so I just dropped back to P2C and stopped when I saw it :-)

Hrm. I fixed those things in the P2C CVS version. Guess I'll have to do a diff to see if there are any other changes... Cheers, -g -- Greg Stein, http://www.lyra.org/

Vladimir Marangozov wrote:

Christian Tismer wrote:

...

About extensions and Trashcan. ... Or, I made a mistake in this little code:

Maybe unrelated, but this code does not handle the case when PyList_Append fails. If it fails, the object needs to be deallocated as usual. Looking at the macros, I don't see how you can do that because Py_TRASHCAN_SAFE_END, which calls the above function, occurs after the finalization code...

Yes, it does not handle this case for the following reasons: Reason 1) If the append does not work, then the system is apparently in a incredibly bad state, most probably broken! Note that these actions only take place when we have a recursion depth of 50 or so. That means, we already freed some memory, and now we have trouble with this probably little list. I won't touch a broken memory management. Reason 2) If the append does not work, then we are not allowed to deallocate the element at all. Trashcan was written in order to avoid crashes for too deeply nested objects. The current nesting level of 20 or 50 is of course very low, but generally I would assume that the limit is choosen for good reasons, and any deeper recursion might cause a machine crash. Under this assumption, the only thing you can do is to forget about the object. Remark ad 1): I had once changed the strategy to use a tuple construct instead. Thinking of memory problems when the shredder list must be grown, this could give an advantage. The optimum would be if the destructor data structure is never bigger than the smallest nested object. This would even allow me to recycle these for the destruction, without any malloc at all. ciao - chris -- Christian Tismer :^) mailto:tismer@appliedbiometrics.com Applied Biometrics GmbH : Have a break! Take a ride on Python's Kaunstr. 26 : *Starship* http://starship.python.net 14163 Berlin : PGP key -> http://wwwkeys.pgp.net PGP Fingerprint E182 71C7 1A9D 66E9 9D15 D3CC D4D7 93E2 1FAE F6DF where do you want to jump today? http://www.stackless.com

Vladimir Marangozov wrote:

Christian Tismer wrote:

...

Vladimir Marangozov wrote:

...

Maybe unrelated, but this code does not handle the case when PyList_Append fails. If it fails, the object needs to be deallocated as usual. Looking at the macros, I don't see how you can do that because Py_TRASHCAN_SAFE_END, which calls the above function, occurs after the finalization code...

Yes, it does not handle this case for the following reasons: ...

Not enough good reasons to segfault. I suggest you move the call to _PyTrash_deposit_object in TRASHCAN_BEGIN and invert the condition there.

Sorry, I don't see what you are suggesting, I'm distracted. Maybe you want to submit a patch, and a few more words on what you mean and why you prefer to core dump with stack overflow? I'm busy seeking a bug in the core, not in that ridiculous code. Somewhere is a real bug, probably the one which I was seeking many time before, when I got weird crashes in the small block heap of Windows. It was never solved, and never clear if it was Python or Windows memory management. Maybe we just found another entrance to this. It smells so very familiar: many many small tuples and we crash. busy-ly y'rs - chris -- Christian Tismer :^) mailto:tismer@appliedbiometrics.com Applied Biometrics GmbH : Have a break! Take a ride on Python's Kaunstr. 26 : *Starship* http://starship.python.net 14163 Berlin : PGP key -> http://wwwkeys.pgp.net PGP Fingerprint E182 71C7 1A9D 66E9 9D15 D3CC D4D7 93E2 1FAE F6DF where do you want to jump today? http://www.stackless.com

Christian Tismer wrote:

Vladimir Marangozov wrote:

...

Not enough good reasons to segfault. I suggest you move the call to _PyTrash_deposit_object in TRASHCAN_BEGIN and invert the condition there.

Sorry, I don't see what you are suggesting, I'm distracted.

I was thinking about the following. Change the macros in object.h from: #define Py_TRASHCAN_SAFE_BEGIN(op) \ { \ ++_PyTrash_delete_nesting; \ if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL) { \ #define Py_TRASHCAN_SAFE_END(op) \ ;} \ else \ _PyTrash_deposit_object((PyObject*)op);\ --_PyTrash_delete_nesting; \ if (_PyTrash_delete_later && _PyTrash_delete_nesting <= 0) \ _PyTrash_destroy_list(); \ } \ to: #define Py_TRASHCAN_SAFE_BEGIN(op) \ { \ ++_PyTrash_delete_nesting; \ if (_PyTrash_delete_nesting >= PyTrash_UNWIND_LEVEL && \ _PyTrash_deposit_object((PyObject*)op) != 0) { \ #define Py_TRASHCAN_SAFE_END(op) \ ;} \ --_PyTrash_delete_nesting; \ if (_PyTrash_delete_later && _PyTrash_delete_nesting <= 0) \ _PyTrash_destroy_list(); \ } \ where _PyTrash_deposit_object returns 0 on success, -1 on failure. This gives another last chance to the system to finalize the object, hoping that the stack won't overflow. :-) My point is that it is better to control whether _PyTrash_deposit_object succeeds or not (and it may fail because of PyList_Append). If this doesn't sound acceptable (because of the possible stack overflow) it would still be better to abort in _PyTrash_deposit_object with an exception "stack overflow on recursive finalization" when PyList_Append fails. Leaving it unchecked is not nice -- especially in such extreme situations. Currently, if something fails, the object is not finalized (leaking memory). Ok, so be it. What's not nice is that this happens silently which is not the kind of tolerance I would accept from the Python runtime. As to the bug: it's curious that, as Mark reported, without the trashcan logic, things seem to run fine. The trashcan seems to provoke (ok, detect ;) some erroneous situation. I'd expect that if the trashcan macros are implemented as above, the crash will go away (which wouldn't solve the problem and would obviate the trashcan in the first place :-) -- Vladimir MARANGOZOV | Vladimir.Marangozov@inrialpes.fr http://sirac.inrialpes.fr/~marangoz | tel:(+33-4)76615277 fax:76615252

Mark, I know you are very busy. But I have no chance to build a debug version, and probably there are more differences. Can you perhaps try Vlad's patch? and tell me if the outcome changes? This would give me much more insight. The change affects the macros and the function _PyTrash_deposit_object which now must report an error via the return value. The macro code should be: #define Py_TRASHCAN_SAFE_BEGIN(op) \ { \ ++_PyTrash_delete_nesting; \ if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL || \ _PyTrash_deposit_object((PyObject*)op) != 0) { \ #define Py_TRASHCAN_SAFE_END(op) \ ;} \ --_PyTrash_delete_nesting; \ if (_PyTrash_delete_later && _PyTrash_delete_nesting <= 0) \ _PyTrash_destroy_list(); \ } \ And the _PyTrash_deposit_object code should be (untested): int _PyTrash_deposit_object(op) PyObject *op; { PyObject *error_type, *error_value, *error_traceback; if (PyThreadState_GET() != NULL) PyErr_Fetch(&error_type, &error_value, &error_traceback); if (!_PyTrash_delete_later) _PyTrash_delete_later = PyList_New(0); if (_PyTrash_delete_later) return PyList_Append(_PyTrash_delete_later, (PyObject *)op); else return -1; if (PyThreadState_GET() != NULL) PyErr_Restore(error_type, error_value, error_traceback); return 0; } The result of this would be really enlighting :-) ciao - chris Vladimir Marangozov wrote:

Of course, this

Vladimir Marangozov wrote:

...

to:

#define Py_TRASHCAN_SAFE_BEGIN(op) \ { \ ++_PyTrash_delete_nesting; \ if (_PyTrash_delete_nesting >= PyTrash_UNWIND_LEVEL && \ _PyTrash_deposit_object((PyObject*)op) != 0) { \

was meant to be this:

#define Py_TRASHCAN_SAFE_BEGIN(op) \ { \ ++_PyTrash_delete_nesting; \ if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL || \ _PyTrash_deposit_object((PyObject*)op) != 0) { \

-- Vladimir MARANGOZOV | Vladimir.Marangozov@inrialpes.fr http://sirac.inrialpes.fr/~marangoz | tel:(+33-4)76615277 fax:76615252

_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://www.python.org/mailman/listinfo/python-dev

-- Christian Tismer :^) mailto:tismer@appliedbiometrics.com Applied Biometrics GmbH : Have a break! Take a ride on Python's Kaunstr. 26 : *Starship* http://starship.python.net 14163 Berlin : PGP key -> http://wwwkeys.pgp.net PGP Fingerprint E182 71C7 1A9D 66E9 9D15 D3CC D4D7 93E2 1FAE F6DF where do you want to jump today? http://www.stackless.com

On Thu, 13 Apr 2000, Mark Hammond wrote:

... I see these possible solutions:

* The trash mechanism is changed to keep a list of (address, deallocator) pairs. This is a "cleaner" solution, as the list is not considered holding PyObjects as such, just blocks of memory to be freed with a custom allocator. Thus, we never end up in a position where a Python objects are resurrected - we just defer the actual memory deallocation, rather than attempting a delayed object destruction. This may not be as trivial to implement as to describe :-)

* Debug builds disable the trash mechanism. Not desired as the basic behaviour of the interpreter will change, making bug tracking with debug builds difficult! If we went this way, I would (try to :-) insist that the Windows debug builds dropped Py_DEBUG, as I really want to avoid the scenario that switching to a debug build changes the behaviour to this extent.

* Perform further hacks, so that Py_ForgetReference() gracefully handles NULL linked-list elements etc.

Any thoughts?

Option 4: lose the trashcan mechanism. I don't think the free-threading issue was ever resolved. Cheers, -g -- Greg Stein, http://www.lyra.org/

On Thu, 13 Apr 2000, Christian Tismer wrote:

Greg Stein wrote: ...

...

Option 4: lose the trashcan mechanism. I don't think the free-threading issue was ever resolved.

Option 5: Forget about free threading, change trashcan in a way that it doesn't change the order of destruction, doesn't need memory at all, and therefore does not change anything if it is disabled in debug mode.

hehe... :-) Definitely possible. Seems like you could just statically allocate an array of PyObject* and drop the pointers in there (without an INCREF or anything). Place them there, in order. Dunno about the debug stuff, and how that would affect it. Cheers, -g -- Greg Stein, http://www.lyra.org/

Vladimir Marangozov wrote:

Christian Tismer wrote:

...

Vladimir Marangozov wrote:

[yup, good looking patch]

where _PyTrash_deposit_object returns 0 on success, -1 on failure. This gives another last chance to the system to finalize the object, hoping that the stack won't overflow. :-)

My point is that it is better to control whether _PyTrash_deposit_object succeeds or not (and it may fail because of PyList_Append). If this doesn't sound acceptable (because of the possible stack overflow) it would still be better to abort in _PyTrash_deposit_object with an exception "stack overflow on recursive finalization" when PyList_Append fails. Leaving it unchecked is not nice -- especially in such extreme situations.

You bet that I *would* raise an exception if I could. Unfortunately the destructors have no way to report an error, and they are always called in a context where no error is expected (Py_DECREF macro). I believe this *was* quite ok, until __del__ was introduced. After that, it looks to me like a design flaw. IMHO there should not be a single function in a system that needs heap memory, and cannot report an error.

Currently, if something fails, the object is not finalized (leaking memory). Ok, so be it. What's not nice is that this happens silently which is not the kind of tolerance I would accept from the Python runtime.

Yes but what can I do? This isn't worse than before. deletion errors die silently, this is the current concept. I don't agree with it, but I'm not the one to change policy. In that sense, trashcan was just compliant to a concept, without saying this is a good concept. :-)

As to the bug: it's curious that, as Mark reported, without the trashcan logic, things seem to run fine. The trashcan seems to provoke (ok, detect ;) some erroneous situation. I'd expect that if the trashcan macros are implemented as above, the crash will go away (which wouldn't solve the problem and would obviate the trashcan in the first place :-)

I think trashcan can be made *way* smarter: Much much more better would be to avoid memory allocation in trashcan at all. I'm wondering if that would be possible. The idea is to catch a couple of objects in an earlier recursion level, and use them as containers for later objects-to-be-deleted. Not using memory at all, that's what I want. And it would avoid all messing with errors in this context. I hate Java dieing silently, since it has not enough memory to tell me that it has not enough memory :-) but-before-implementing-this-*I*-will-need-to-become-*way*-smarter - ly y'rs - chris -- Christian Tismer :^) mailto:tismer@appliedbiometrics.com Applied Biometrics GmbH : Have a break! Take a ride on Python's Kaunstr. 26 : *Starship* http://starship.python.net 14163 Berlin : PGP key -> http://wwwkeys.pgp.net PGP Fingerprint E182 71C7 1A9D 66E9 9D15 D3CC D4D7 93E2 1FAE F6DF where do you want to jump today? http://www.stackless.com