[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[STINNER Victor]

STINNER Victor  added the comment:

> tp_traverse is optional, so we should not add this assertion.

It is required by types implementing the GC protocol. This assumption is 
non-obvious and was not documented. The documentation was completed in 
bpo-44263 by commit 8b55bc3f93a655bc803bff79725d5fe3f124e2f0.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[STINNER Victor]

STINNER Victor  added the comment:

This issue was fixed differently in bpo-44263, by adding a test in 
PyType_Ready():

// bpo-44263: tp_traverse is required if Py_TPFLAGS_HAVE_GC is set.
// Note: tp_clear is optional.
if (type->tp_flags & Py_TPFLAGS_HAVE_GC
&& type->tp_traverse == NULL)
{
PyErr_Format(PyExc_SystemError,
 "type %s has the Py_TPFLAGS_HAVE_GC flag "
 "but has no traverse function",
 type->tp_name);
return -1;
}

commit ee7637596d8de25f54261bbeabc602d31e74f482
Author: Victor Stinner 
Date:   Tue Jun 1 23:37:12 2021 +0200

bpo-44263: Py_TPFLAGS_HAVE_GC requires tp_traverse (GH-26463)

The PyType_Ready() function now raises an error if a type is defined
with the Py_TPFLAGS_HAVE_GC flag set but has no traverse function
(PyTypeObject.tp_traverse).

--
nosy: +vstinner
resolution: rejected -> fixed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Irit Katriel]

Irit Katriel  added the comment:

tp_traverse is optional, so we should not add this assertion.

--
resolution:  -> rejected
stage: test needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Bryan Silverthorn]

Bryan Silverthorn  added the comment:

I submitted this patch 14 years ago and am sure of nothing. :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Irit Katriel]

Irit Katriel  added the comment:

Are you sure this assertion is correct? tp_traverse is optional.

--
nosy: +iritkatriel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 3.2, Python 3.3 -Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1662
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

It would be probably be better to put this check in PyType_Ready() instead.

--
nosy: +pitrou
versions: +Python 2.7, Python 3.1 -Python 2.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1662
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Daniel Diniz]

Daniel Diniz aja...@gmail.com added the comment:

Bryan: can you provide a test case without external dependencies? If
not, confirming this is still valid in 2.6 would also help.

--
nosy: +ajaksu2
stage:  - test needed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1662
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Bryan Silverthorn]

Bryan Silverthorn bc...@cornell.edu added the comment:

Well, there's no Python bug per se, hence no test case; this patch just
adds a single additional assert that might catch a particular extension
implementation mistake. It was prompted by tracking down the bug in
pygtk mentioned above.

I've attached an updated patch against r72055. It's a trivial change,
but I would suggest that someone more familiar with the Python core sign
off on it regardless.

--
Added file: http://bugs.python.org/file13800/bcs_assert_tp_traverse_r72055.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1662
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Bryan Silverthorn]

Changes by Bryan Silverthorn bc...@cornell.edu:


Removed file: http://bugs.python.org/file8998/bcs_typeobject_assert.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1662
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Bryan Silverthorn]

New submission from Bryan Silverthorn:

Attached is a very short patch against r59568 which asserts tp_traverse
on (the types of) objects allocated in PyType_GenericAlloc(). As far as
I'm aware, tp_traverse should always be set at this point. Catching that
error early, even if only in debug builds, would help to prevent bugs
like http://bugzilla.gnome.org/show_bug.cgi?id=504337 .

--
components: Interpreter Core
files: bcs_typeobject_assert.patch
messages: 58811
nosy: bsilverthorn
severity: minor
status: open
title: [patch] assert tp_traverse in PyType_GenericAlloc()
type: behavior
versions: Python 2.6
Added file: http://bugs.python.org/file8998/bcs_typeobject_assert.patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1662
__Index: Objects/typeobject.c
===
--- Objects/typeobject.c	(revision 59568)
+++ Objects/typeobject.c	(working copy)
@@ -469,8 +469,10 @@
 	else
 		(void) PyObject_INIT_VAR((PyVarObject *)obj, type, nitems);
 
-	if (PyType_IS_GC(type))
+	if (PyType_IS_GC(type)) {
+		assert(type-tp_traverse);
 		_PyObject_GC_TRACK(obj);
+	}
 	return obj;
 }
 
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1662] [patch] assert tp_traverse in PyType_GenericAlloc()

[Christian Heimes]

Changes by Christian Heimes:


--
keywords: +patch
priority:  - normal

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue1662
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com