[Python-bugs-list] [Bug #115076] threads of __del__

noreply@sourceforge.net noreply@sourceforge.net
Fri, 22 Sep 2000 11:52:35 -0700


Bug #115076, was updated on 2000-Sep-22 02:15
Here is a current snapshot of the bug.

Project: Python
Category: Core
Status: Open
Resolution: None
Bug Group: None
Priority: 3
Summary: threads of __del__

Details: I suspect (but have not proved) that the support for destroying deeply recursive structures can allow for an instances __del__ method to get called in a thread other than the one in which it lost its last reference.

This can happen if it switches to a different thread during deletion of a deeply recursive structure (with _PyTrash_delete_later not empty). 

Should Python guarantee that this does not happen? (I say yes - it is important for COM clients)

Follow-Ups:

Date: 2000-Sep-22 02:52
By: gvanrossum

Comment:
Agreed that this would be a problem if it could cause an inconsistent state of the trashcan (at the end of object.c) structure. However, since the "trash" code is all executed with the global interpreter lock held, I don't see that there can be a problem. Here's why:

The code in _PyTrash_deposit_object() doesn't make any calls that could invoke __del__, so it's safe. The code in _PyTrash_destroy_chain() makes exactly one call to Py_DECREF(), but it takes care that its global data structures are consistent at this point. For a second I thought that _PyTrash_delete_later would have to be declared volatile, but on second thought I don't: the Py_DECREF() macro expands to a possible (indirect) function call, signaling the compiler that the global _PyTrash_delete_later may be changed in arbitrary ways.

Assigned to Tim Peters to verify my reasoning, because of his superior mind in reasoning about threads and race conditions.
-------------------------------------------------------

Date: 2000-Sep-22 03:05
By: htrd

Comment:
I agree there is no self-consistency problem with the trashcan, however......

The code in _PyTrash_destroy_chain can call an objects __del__ method, which can release the interpreter lock.

A second thread may then reenter _PyTrash_destroy_chain and continue to clean up the first thread's garbage. (the first thread is safely blocked inside the Py_DECREF)

Yes the garbage gets cleaned up, however it happens in the wrong thread.

I suggest that _PyTrash_delete_later should be part of pyThreadState



-------------------------------------------------------

Date: 2000-Sep-22 03:08
By: gvanrossum

Comment:
And why, exactly, would it be a problem that the trash is deleted in the 'wrong" thread? It's just trash. Any thread can delete it.

Or am I missing something?
-------------------------------------------------------

Date: 2000-Sep-22 03:27
By: htrd

Comment:
There are two cases that I have in mind:

First is that of a COM client.

It depends on the COM threading model in use, but this problem applies in the most common threading model which is also the default threading model using Mark Hammond's pythoncom.

It is only permitted to access a reference to a COM object from the thread in which it is created. This restiction applies to the Release() method that is used by pythoncom to release its reference to the COM server.


Secondly, it is common to use a dictionary keyed by thread.get_ident() to provide thread-local storage. This is used extensively in Zope. Any application that uses this type of thread-local storage in a __del__ is likely to be broken.



-------------------------------------------------------

Date: 2000-Sep-22 06:30
By: gvanrossum

Comment:
It sounds that it will be hard to enforce that __del__ is called in the constructing thread even without the complication of the trashcan code.

Explicitly referencing thread-specific data in a __del__ seems silly -- if the object is tied so closely to the thread, why not add a pointer to the thread to the object.

Nevertheless, especially the COM thing sounds like it would be a good thing to prevent this if it is possible, and your solution (make the trashcan state part of the thread state) sounds reasonable -- except that it means that every deallocation of a tuple, list, dict, frame or traceback needs yet another call to PyThreadState_Get(). And, most disturbing of all, we found before releasing 2.0b1 that there are situations where dictionaries (and possibly other objects) are used when there is no current thread state!!!

Replacing one theoretical problem with another not-so-theoretical one doesn't seem right. Perhaps we could invent a flag that prevents thread-switches while the trashcan dealloc code is running? This could be a simple global boolean that, when set, stops the ceval main loop (and the BEGIN/END ALLOW THREADS macros and related functions!) from ever yielding the interpreter loop. It would almost never be triggered in practice, but guarantee safety in worst cases -- if not efficiency (other threads would be completely blocked until the trashcan code is done).

Back to Tim...
-------------------------------------------------------

Date: 2000-Sep-22 07:52
By: htrd

Comment:
There is a patch to move the trashcan into the thread state. see http://sourceforge.net/patch/?func=detailpatch&patch_id=101606&group_id=5470

This uses PyThreadState_GET() once to avoid the extra function call in the normal path, and only uses the trashcan if the thread state is non NULL. (assuming we are never deleting deeply recursive objects with no thread state)

On performance, this gives me a 2% increase in pystones....

-------------------------------------------------------

Date: 2000-Sep-22 11:52
By: tim_one

Comment:
Assigned to MarkH, as I want his opinion on this.

I dislike it on principle:  it elevates an accident of refcount semantics into a design goal, and complicates the code to achieve that.

The Microsoft .NET implementation of Python doesn't do refcounting, so not even the pre-trashcan refcount semantics can be relied on there, and I don't want to add to users' difficulties in moving among Python implementations (despite that it may be in my immediate financial interest to lock people into CPython, I'll leave "embrace & extend" to MS <wink>).

If this sounds like a good idea to Mark anyway, fine, but I don't buy it otherwise.
-------------------------------------------------------

For detailed info, follow this link:
http://sourceforge.net/bugs/?func=detailbug&bug_id=115076&group_id=5470