Make memoryview weakrefable

[sbt]

BPO	14930
Nosy	@jcea, @pitrou, @skrah
Files	memoryview-weakref.patch memoryview-weakref.patch memoryview-weakref.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-05-29.08:32:17.858>
created_at = <Date 2012-05-27.21:05:59.838>
labels = ['interpreter-core', 'type-feature']
title = 'Make memoryview weakrefable'
updated_at = <Date 2012-07-25.17:03:32.019>
user = 'https://bugs.python.org/sbt'

bugs.python.org fields:

activity = <Date 2012-07-25.17:03:32.019>
actor = 'skrah'
assignee = 'none'
closed = True
closed_date = <Date 2012-05-29.08:32:17.858>
closer = 'sbt'
components = ['Interpreter Core']
creation = <Date 2012-05-27.21:05:59.838>
creator = 'sbt'
dependencies = []
files = ['25739', '25742', '25744']
hgrepos = []
issue_num = 14930
keywords = ['patch']
message_count = 14.0
messages = ['161729', '161730', '161763', '161770', '161776', '161778', '161779', '161788', '161797', '161806', '166381', '166406', '166412', '166416']
nosy_count = 6.0
nosy_names = ['jcea', 'pitrou', 'skrah', 'neologix', 'python-dev', 'sbt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14930'
versions = ['Python 3.3']

[sbt]

The attached patch makes memoryview objects weakrefable.

The reason I would like them to be weakrefable is so that I can manage the finalization and pickling of memoryview objects which wrap shared mmap segments.

(It would be even better if memoryview were subclassable, but I don't know if naively changing tp_flags would be enough.)

[pitrou]

Looks like an obviously good idea.
The patch needs tests, though.

[sbt]

New patch.

[pitrou]

In the test, you should call gc.collect() so that it works on non-reference counted implementations. Also, I would call PyObject_ClearWeakRefs() after memory_release() and Py_CLEAR(self->mbuf), not before (in case a weakref callback relies on the buffer being released).

[sbt]

In the test, you should call gc.collect() so that it works on non-
reference counted implementations.

I did think about using gc.collect(), but I was not sure whether it was guaranteed to collect everything possible if you only call it only once. (I know nothing about garbage collectors.)

Also, I would call PyObject_ClearWeakRefs() after memory_release() and
Py_CLEAR(self->mbuf), not before (in case a weakref callback relies on
the buffer being released).

Doing it after Py_CLEAR(self->mbuf) seems to contradict

http://docs.python.org/dev/extending/newtypes.html?highlight=pyobject_clearweakrefs#weak-reference-support

which says

The only further addition is that the destructor needs to call the weak 
reference manager to clear any weak references. This should be done *before*
any other parts of the destruction have occurred, but is only required if the 
weak reference list is non-NULL:

[neologix]

I did think about using gc.collect(), but I was not sure whether it was
guaranteed to collect everything possible if you only call it only once.
(I know nothing about garbage collectors.)

You could use support.gc_collect() for that.

[pitrou]

Doing it after Py_CLEAR(self->mbuf) seems to contradict

http://docs.python.org/dev/extending/newtypes.html?highlight=pyobject_clearweakrefs#weak-reference-support

which says

The only further addition is that the destructor needs to call the weak 
reference manager to clear any weak references. This should be done \*before\*
any other parts of the destruction have occurred, but is only required if the 
weak reference list is non-NULL:

Mmh, this seems to be misled. The original formulation is from SVN
r16381; the clearly erroneous part about resurrecting objects was later
removed in r18223, but the rest is probably unnecessary as well. I'll
open a separate issue.

[sbt]

Updated patch.

[pitrou]

Updated patch.

Looks good to me!

[python-dev]

New changeset bc0281f85409 by Richard Oudkerk in branch 'default':
Issue bpo-14930: Make memoryview objects weakrefable.
http://hg.python.org/cpython/rev/bc0281f85409

[skrah]

A quick question: Prior to this patch test_memoryview.py exercised
both mbuf_clear() and memory_clear(). Now gcov shows no coverage.

Is this expected? Is it still possible to construct tests that
exercise the code?

[sbt]

Is it possible that the use of test.support.gc_collect() in test_memoryview made the difference?