This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Crash due to using weakref referent without acquiring a strong reference
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: duplicate
Dependencies: Superseder: proxy_contains (weakref.proxy) can access an object with 0 refcount
View: 38395
Assigned To: Nosy List: arigo, ldeller
Priority: normal Keywords: patch

Created on 2015-11-30 05:16 by ldeller, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue25769-weakref.patch ldeller, 2015-11-30 06:06 Patch (against hg default branch = 3.6) review
Messages (4)
msg255608 - (view) Author: lplatypus (ldeller) * Date: 2015-11-30 05:16
I have encountered some crashes in a multithreaded application which appear to be due to a bug in weakref_richcompare in Objects/weakref.c

(I am using Python 2.7.9, but the same weakref code exists in 3.5 and hg default branch too)

weakref_richcompare ends with the statement:

    return PyObject_RichCompare(PyWeakref_GET_OBJECT(self),
                                PyWeakref_GET_OBJECT(other), op);

At this point the code has established that the referents of "self" and "other" are still alive, and it is trying to compare the referents.  However it has not acquired a strong reference to the referents, so I think it is possible for one of them to be deleted half way through this comparison.  This can lead to a crash, because PyObject_RichCompare assumes that the PyObject*’s it was passed will remain usable for the duration of the call.

The crash dumps I have seen involve data corruption consistent with one of these PyObject's being deleted and the memory used for something else, eg:

00 python27!try_3way_compare+0x15 [objects\object.c @ 712]
01 python27!try_3way_to_rich_compare+0xb [objects\object.c @ 901]
02 python27!do_richcmp+0x2c [objects\object.c @ 935]
03 python27!PyObject_RichCompare+0x99 [objects\object.c @ 982]
04 python27!weakref_richcompare+0x6a [objects\weakrefobject.c @ 212]

In this example, in try_3way_compare the value of v->ob_type was 0x5f637865, which is ASCII "exc_" and not a valid pointer at all.

Other places in weakrefobject.c seem to have a similar weakness, eg in weakref_hash and weakref_repr.

I have not been successful in producing a small test case to demonstrate this crash.
msg255610 - (view) Author: lplatypus (ldeller) * Date: 2015-11-30 06:06
I think the fix for this is simply a matter of using Py_INCREF/Py_DECREF around usage of the referent.  This should only be necessary for nontrivial usages where the GIL might be released.  Here is a patch.
msg255617 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-11-30 10:49
PyWeakref_GET_OBJECT() is inherently dangerous: the weakref might go away not only the next time the GIL is released, but also the next time the code does any seemingly unrelated Py_DECREF() (which might decref the object into inexistence) or memory allocation (which might trigger the GC).

I'd suggest adding the comment above in the docs and reviewing all usages inside CPython's code base.
msg255619 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-11-30 13:28
Actually, Py_DECREF() may also release the GIL by calling a Python __del__() method; it doesn't have to directly decref the exact object.
History
Date User Action Args
2022-04-11 14:58:24adminsetgithub: 69955
2021-09-07 15:48:05iritkatrielsetstatus: open -> closed
superseder: proxy_contains (weakref.proxy) can access an object with 0 refcount
resolution: duplicate
stage: resolved
2020-11-06 17:40:19iritkatrielsetversions: + Python 3.8, Python 3.9, Python 3.10, - Python 2.7, Python 3.2, Python 3.3, Python 3.4, Python 3.5, Python 3.6
2015-11-30 13:28:31arigosetmessages: + msg255619
2015-11-30 10:49:24arigosetnosy: + arigo
messages: + msg255617
2015-11-30 06:06:53ldellersetfiles: + issue25769-weakref.patch
keywords: + patch
messages: + msg255610
2015-11-30 05:16:04ldellercreate