[issue14369] make __closure__ writable

[STINNER Victor]

New submission from Yury Selivanov <[hidden email]>:

__code__ and __closure__ are designed to work together.  There is no point in allowing to completely substitute the __code__ object, but protecting the __closure__.

----------
components: Interpreter Core
files: writable_closure.patch
keywords: patch
messages: 156350
nosy: Yury.Selivanov, asvetlov, pitrou
priority: normal
severity: normal
status: open
title: make __closure__ writable
type: enhancement
versions: Python 3.3
Added file: http://bugs.python.org/file24943/writable_closure.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Changes by Yury Selivanov <[hidden email]>:


Removed file: http://bugs.python.org/file24943/writable_closure.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Changes by Yury Selivanov <[hidden email]>:


Added file: http://bugs.python.org/file24946/writable_closure.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Yury Selivanov <[hidden email]> added the comment:

Updated patch as per Andrew's code review.  Thank you.

----------
Added file: http://bugs.python.org/file24947/writable_closure_02.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Andrew Svetlov <[hidden email]> added the comment:

Please update the doc also. I think changing from 'Read-only' to 'Writable' in Doc/reference/datamodel.rst is enough.

----------

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Yury Selivanov <[hidden email]> added the comment:

> Please update the doc also. I think changing from 'Read-only' to 'Writable' in Doc/reference/datamodel.rst is enough.

Updated in writable_closure_03.patch.  Thanks.

----------
Added file: http://bugs.python.org/file24962/writable_closure_03.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Changes by Andrew Svetlov <[hidden email]>:


----------
stage:  -> patch review

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Changes by Andrew Svetlov <[hidden email]>:


----------
nosy: +benjamin.peterson

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Yury Selivanov <[hidden email]> added the comment:

Updated patch per Benjamin's review. See writable_closure_04.patch.

----------
Added file: http://bugs.python.org/file24975/writable_closure_04.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Changes by Yury Selivanov <[hidden email]>:


----------
nosy: +ncoghlan

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Nick Coghlan <[hidden email]> added the comment:

Another use case for a writeable __closure__ attribute is to make it possible to manually break reference cycles:
http://blog.ccpgames.com/kristjan/2012/04/23/reference-cycles-with-closures/

----------

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Richard Oudkerk <[hidden email]> added the comment:

Shouldn't test___closure__() also test what happens when the closure is replaced with None, or a tuple which is too long or too short or contains non-cell objects?

All of these things seem to be checked when you create a new function using types.FunctionType:

>>> h = types.FunctionType(g.__code__, g.__globals__, "h", g.__defaults__, None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: arg 5 (closure) must be tuple
>>> h = types.FunctionType(g.__code__, g.__globals__, "h", g.__defaults__, ())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: g requires closure of length 2, not 0
>>> h = types.FunctionType(g.__code__, g.__globals__, "h", g.__defaults__, (1,2))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: arg 5 (closure) expected cell, found int

I think the setter should make similar checks.  Maybe there is C code which assumes "broken" closures never happen.

----------
nosy: +sbt

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Richard Oudkerk <[hidden email]> added the comment:

The patch causes crashes.  If I define

  def cell(o):
    def f(): o
    return f.__closure__[0]

  def f():
    a = 1
    b = 2
    def g():
      return a + b
    return g

  g = f()

then I find

  g.__closure__ = None; g()                          -> crash
  g.__closure__ = (cell(3),); g()                    -> crash
  g.__closure__ = (1, 2); g()                        -> SystemError *
  g.__closure__ = (cell(3), cell(4), cell(5)); g()   -> returns 7

* SystemError: ..\Objects\cellobject.c:24: bad argument to internal function

----------

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Yury Selivanov <[hidden email]> added the comment:

> The patch causes crashes.

Yes, that's known.  

First, we need to check, that we can only write tuple of cell objects or None in __closure__ (that's easy to add).  Secondly, perhaps, we can check __closure__ correctness each time we start evaluating a code object.  The latter would offer us better stability, but may also introduce some slowdowns -- need to find some time to implement and benchmark this.

----------

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Richard Oudkerk <[hidden email]> added the comment:

Version of patch which checks invariants in the setter and adds tests.

----------
Added file: http://bugs.python.org/file25363/writable_closure_with_checking.patch

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com

[STINNER Victor]

Andrew Svetlov <[hidden email]> added the comment:

sbt, looks good for me.

----------

_______________________________________
Python tracker <[hidden email]>
<http://bugs.python.org/issue14369>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/lists%2B1322467933539-512619%40n6.nabble.com