[Pyrex] [Cython-dev] fix for tp_clear()

[Robert Bradshaw]

On Jan 12, 2008, at 2:23 PM, Stefan Behnel wrote:

> Hi,
>
> I happened to read this page, which describes how tp_clear() should be
> implemented.
>
> http://docs.python.org/ext/node24.html
>
> The race condition that is described there and in the comment in
> Python/Include/object.h hits Pyrex and Cython (although I can't  
> tell how rare
> it really is - I never noticed anything like that).
>
> Also, Pyrex and Cython set the type members to None, while the page  
> above
> assigns NULL. I don't know if None is needed here - to mimic the old
> behaviour, you could do the usual
>
>     "tmp = %s; %s = Py_None; Py_INCREF(Py_None);" % (name, name)
>
> dance instead of the
>
>     "tmp = %s; %s = 0;" % (name, name)
>
> INCREFs are cheap, so this won't matter too much performance-wise. The
> important bit is that the pointer gets cleared *before* the final  
> XDECREF(),
> which may lead to the execution of arbitrary code (such as deallocs  
> and whatnot).
>
> I'd like to hear opinions why the None would be necessary here,  
> otherwise I'd
> just leave it with the NULL.

I believe this is so the object starts out in a valid state, and we  
never have to worry (e.g. elsewhere in the code)  that members do not  
contain a valid Python object.

> Stefan
> # HG changeset patch
> # User Stefan Behnel <scoder at users.berlios.de>
> # Date 1200170010 -3600
> # Node ID bac16e6f5023d048cb933fbf6b3a5e181e42b9f8
> # Parent  570ab39b02fb348018ec78fe75947ed0cae72321
> fix tp_clear() according to Python docs: zero out member pointer  
> before decrefing its content
>
> diff -r 570ab39b02fb -r bac16e6f5023 Compiler/ModuleNode.py
> --- a/Compiler/ModuleNode.py	Sat Jan 12 14:45:03 2008 +0100
> +++ b/Compiler/ModuleNode.py	Sat Jan 12 21:33:30 2008 +0100
> @@ -758,6 +758,7 @@ class ModuleNode(Nodes.Node, Nodes.Block
>                  py_attrs.append(entry)
>          if py_attrs:
>              self.generate_self_cast(scope, code)
> +            code.putln("PyObject* tmp;")
>          if base_type:
>              code.putln("if (%s->tp_clear) {" %  
> base_type.typeptr_cname)
>              code.putln(
> @@ -766,8 +767,8 @@ class ModuleNode(Nodes.Node, Nodes.Block
>              code.putln("}")
>          for entry in py_attrs:
>              name = "p->%s" % entry.cname
> -            code.put_xdecref(name, entry.type)
> -            code.put_init_var_to_py_none(entry, "p->%s")
> +            code.putln("tmp = %s; %s = 0;" % (name, name))
> +            code.putln("Py_XDECREF(tmp);")
>          code.putln(
>              "return 0;")
>          code.putln(
> _______________________________________________
> Cython-dev mailing list
> Cython-dev at lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/cython-dev