Skip to content

Commit 282887e

Browse files
committed
Fix segfaults accessing _OpenSlideMap after OpenSlide released
Rework destructor and close() handling to ensure that: - Operations on an _OpenSlideMap object don't crash if its parent OpenSlide object has been freed. - Operations on an _OpenSlideMap object raise an exception, rather than crashing, if the parent OpenSlide object has been explicitly closed. - Freeing an OpenSlide object during interpreter shutdown correctly releases the OpenSlide handle. Closes #6.
1 parent 641ab2e commit 282887e

File tree

2 files changed

+18
-6
lines changed

2 files changed

+18
-6
lines changed

openslide/__init__.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,6 @@ def __init__(self, filename):
144144
AbstractSlide.__init__(self)
145145
self._osr = lowlevel.open(filename)
146146

147-
def __del__(self):
148-
if getattr(self, '_osr', None) is not None:
149-
self.close()
150-
151147
@classmethod
152148
def can_open(cls, filename):
153149
"""Return True if OpenSlide can read the specified file."""
@@ -156,7 +152,6 @@ def can_open(cls, filename):
156152
def close(self):
157153
"""Close the OpenSlide object."""
158154
lowlevel.close(self._osr)
159-
self._osr = None
160155

161156
@property
162157
def layer_count(self):

openslide/lowlevel.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,26 @@ class _OpenSlide(object):
5151

5252
def __init__(self, ptr):
5353
self._as_parameter_ = ptr
54+
self._valid = True
55+
# Retain a reference to close() to avoid GC problems during
56+
# interpreter shutdown
57+
self._close = close
58+
59+
def __del__(self):
60+
if self._valid:
61+
self._close(self)
62+
63+
def invalidate(self):
64+
self._valid = False
5465

5566
@classmethod
5667
def from_param(cls, obj):
5768
if obj.__class__ != cls:
5869
raise ValueError("Not an OpenSlide reference")
5970
if not obj._as_parameter_:
6071
raise ValueError("Passing undefined slide object")
72+
if not obj._valid:
73+
raise ValueError("Passing closed slide object")
6174
return obj
6275

6376
# check for errors opening an image file and wrap the resulting handle
@@ -66,6 +79,10 @@ def _check_open(result, _func, _args):
6679
raise OpenSlideError("Could not open image file")
6780
return _OpenSlide(result)
6881

82+
# prevent further operations on slide handle after it is closed
83+
def _check_close(_result, _func, args):
84+
args[0].invalidate()
85+
6986
# check if the library got into an error state after each library call
7087
def _check_error(result, _func, args):
7188
err = get_error(args[0])
@@ -108,7 +125,7 @@ def _load_image(buf, size):
108125

109126
open = _func('openslide_open', c_void_p, [c_char_p], _check_open)
110127

111-
close = _func('openslide_close', None, [_OpenSlide], None)
128+
close = _func('openslide_close', None, [_OpenSlide], _check_close)
112129

113130
get_layer_count = _func('openslide_get_layer_count', c_int32, [_OpenSlide])
114131

0 commit comments

Comments
 (0)