Skip to content

Commit d7608fd

Browse files
committed
BUG: Disown Python objects adopted by unique_ptr members
GradientImageFilter::OverrideBoundaryCondition and OptimizerParameters::SetHelper store their raw-pointer argument in a unique_ptr member, but the Python wrapping left ownership with the proxy. Both sides freed the object and the interpreter aborted at shutdown. Apply SWIGTYPE *DISOWN to both parameters and add Python regression tests.
1 parent 3ae346c commit d7608fd

6 files changed

Lines changed: 114 additions & 0 deletions

File tree

Modules/Core/Common/wrapping/itkOptimizerParameters.wrap

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ itk_end_wrap_class()
77

88
itk_wrap_class("itk::OptimizerParameters")
99
foreach(t ${types})
10+
# SetHelper stores the argument in a unique_ptr member.
11+
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "%apply SWIGTYPE *DISOWN { itkOptimizerParametersHelper${ITKM_${t}} * helper };\n")
12+
1013
itk_wrap_template("${ITKM_${t}}" "${ITKT_${t}}")
1114
endforeach()
1215
itk_end_wrap_class()

Modules/Core/Common/wrapping/test/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,9 @@ if(ITK_WRAP_PYTHON)
6666
COMMAND
6767
${CMAKE_CURRENT_SOURCE_DIR}/itkImageLifetimeTest.py
6868
)
69+
itk_python_add_test(
70+
NAME itkOptimizerParametersOwnershipPythonTest
71+
COMMAND
72+
${CMAKE_CURRENT_SOURCE_DIR}/itkOptimizerParametersOwnershipTest.py
73+
)
6974
endif()
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# ==========================================================================
2+
#
3+
# Copyright NumFOCUS
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# https://www.apache.org/licenses/LICENSE-2.0.txt
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
#
17+
# ==========================================================================
18+
"""OptimizerParameters.SetHelper takes ownership of its argument.
19+
20+
Without a DISOWN typemap, Python and the m_Helper unique_ptr both free the helper,
21+
and the interpreter aborts at shutdown. A non-zero exit code from this script is
22+
itself part of the regression check.
23+
"""
24+
25+
import itk
26+
27+
parameters = itk.OptimizerParameters[itk.D](3)
28+
helper = itk.OptimizerParametersHelper[itk.D]()
29+
30+
assert helper.thisown, "Python should own a freshly constructed helper"
31+
32+
parameters.SetHelper(helper)
33+
34+
assert not helper.thisown, "SetHelper must transfer ownership away from Python"
35+
36+
print("Test finished.")

Modules/Filtering/ImageGradient/wrapping/itkGradientImageFilter.wrap

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ foreach(d ${ITK_WRAP_IMAGE_DIMS})
44
set(vector_dim ${d}) # Wrap only vector dimensions which are the same as image dimensions
55
foreach(t ${WRAP_ITK_SCALAR})
66

7+
# OverrideBoundaryCondition stores the argument in a unique_ptr member.
8+
string(
9+
APPEND
10+
ITK_WRAP_PYTHON_SWIG_EXT
11+
"%apply SWIGTYPE *DISOWN { itkImageBoundaryCondition${ITKM_I${t}${vector_dim}} * boundaryCondition };\n"
12+
)
13+
714
if(ITK_WRAP_covariant_vector_float)
815
itk_wrap_template("${ITKM_I${t}${vector_dim}}${ITKM_F}${ITKM_F}" "${ITKT_I${t}${vector_dim}},${ITKT_F},${ITKT_F}")
916
endif()

Modules/Filtering/ImageGradient/wrapping/test/CMakeLists.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,10 @@ if(ITK_WRAP_PYTHON AND ITK_WRAP_float AND wrap_2_index GREATER -1)
1414
${ITK_TEST_OUTPUT_DIR}/GradientMagnitudeRecursiveGaussianImageFilterTest.png
1515
5
1616
)
17+
18+
itk_python_add_test(
19+
NAME itkGradientImageFilterOwnershipPythonTest
20+
COMMAND
21+
${CMAKE_CURRENT_SOURCE_DIR}/itkGradientImageFilterOwnershipTest.py
22+
)
1723
endif()
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# ==========================================================================
2+
#
3+
# Copyright NumFOCUS
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# https://www.apache.org/licenses/LICENSE-2.0.txt
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
#
17+
# ==========================================================================
18+
"""GradientImageFilter.OverrideBoundaryCondition takes ownership of its argument.
19+
20+
Without a DISOWN typemap, Python and the filter's unique_ptr member both free the
21+
boundary condition, and the interpreter aborts at shutdown. A non-zero exit code
22+
from this script is itself part of the regression check.
23+
"""
24+
25+
import itk
26+
27+
itk.auto_progress(2)
28+
29+
ImageType = itk.Image[itk.F, 2]
30+
31+
filt = itk.GradientImageFilter[ImageType, itk.F, itk.F].New()
32+
boundaryCondition = itk.PeriodicBoundaryCondition[ImageType]()
33+
34+
assert (
35+
boundaryCondition.thisown
36+
), "Python should own a freshly constructed boundary condition"
37+
38+
filt.OverrideBoundaryCondition(boundaryCondition)
39+
40+
assert (
41+
not boundaryCondition.thisown
42+
), "OverrideBoundaryCondition must transfer ownership away from Python"
43+
44+
# The filter must remain usable with the adopted boundary condition.
45+
image = ImageType.New()
46+
region = itk.ImageRegion[2]()
47+
region.SetSize([8, 8])
48+
image.SetRegions(region)
49+
image.Allocate()
50+
image.FillBuffer(1.0)
51+
52+
filt.SetInput(image)
53+
filt.Update()
54+
55+
assert filt.GetOutput().GetLargestPossibleRegion().GetSize()[0] == 8
56+
57+
print("Test finished.")

0 commit comments

Comments
 (0)