Commit a40f34e0 authored by da-woods's avatar da-woods Committed by GitHub

Fix reference counting in loops over memoryviews (GH-4663)

Memoryview temps weren't being properly incremented (at the start
of the loop) or decremented (at the end of the loop). They were,
however, decremented in the exception handling case.

Long-term I think we really should unify reference counting for
memoryviews further. It's *slightly* different from PyObjects
(mainly that it happens in wrapper functions rather than the
main function) and that leads to a large chance that it just
gets missed.

Fixes https://github.com/cython/cython/issues/4662
parent 612e985f
...@@ -233,7 +233,10 @@ class LetNodeMixin: ...@@ -233,7 +233,10 @@ class LetNodeMixin:
if self._result_in_temp: if self._result_in_temp:
self.temp = self.temp_expression.result() self.temp = self.temp_expression.result()
else: else:
self.temp_expression.make_owned_reference(code) if self.temp_type.is_memoryviewslice:
self.temp_expression.make_owned_memoryviewslice(code)
else:
self.temp_expression.make_owned_reference(code)
self.temp = code.funcstate.allocate_temp( self.temp = code.funcstate.allocate_temp(
self.temp_type, manage_ref=True) self.temp_type, manage_ref=True)
code.putln("%s = %s;" % (self.temp, self.temp_expression.result())) code.putln("%s = %s;" % (self.temp, self.temp_expression.result()))
...@@ -246,7 +249,7 @@ class LetNodeMixin: ...@@ -246,7 +249,7 @@ class LetNodeMixin:
self.temp_expression.generate_disposal_code(code) self.temp_expression.generate_disposal_code(code)
self.temp_expression.free_temps(code) self.temp_expression.free_temps(code)
else: else:
if self.temp_type.is_pyobject: if self.temp_type.needs_refcounting:
code.put_decref_clear(self.temp, self.temp_type) code.put_decref_clear(self.temp, self.temp_type)
code.funcstate.release_temp(self.temp) code.funcstate.release_temp(self.temp)
......
...@@ -2516,3 +2516,48 @@ def test_const_buffer(const int[:] a): ...@@ -2516,3 +2516,48 @@ def test_const_buffer(const int[:] a):
cdef const int[:] c = a cdef const int[:] c = a
print(a[0]) print(a[0])
print(c[-1]) print(c[-1])
@testcase
def test_loop(int[:] a, throw_exception):
"""
>>> A = IntMockBuffer("A", range(6), shape=(6,))
>>> test_loop(A, False)
acquired A
15
released A
>>> try:
... test_loop(A, True)
... except ValueError:
... pass
acquired A
released A
"""
cdef int sum = 0
for ai in a:
sum += ai
if throw_exception:
raise ValueError()
print(sum)
@testcase
def test_loop_reassign(int[:] a):
"""
>>> A = IntMockBuffer("A", range(6), shape=(6,))
>>> test_loop_reassign(A)
acquired A
0
1
2
3
4
5
released A
15
"""
cdef int sum = 0
for ai in a:
sum += ai
print(ai)
a = None # this should not mess up the loop though!
# release happens here, when the loop temp is released
print(sum)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment