Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sf_open error checking when used concurrently #279

Merged
merged 4 commits into from Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 12 additions & 4 deletions soundfile.py
Expand Up @@ -1181,8 +1181,10 @@ def _open(self, file, mode_int, closefd):
mode_int, self._info, _ffi.NULL)
else:
raise TypeError("Invalid file: {0!r}".format(self.name))
_error_check(_snd.sf_error(file_ptr),
"Error opening {0!r}: ".format(self.name))
if file_ptr == _ffi.NULL:
# get the actual error code
err = _snd.sf_error(file_ptr)
raise LibsndfileError(err, prefix="Error opening {0!r}: ".format(self.name))
if mode_int == _snd.SFM_WRITE:
# Due to a bug in libsndfile version <= 1.0.25, frames != 0
# when opening a named pipe in SFM_WRITE mode.
Expand Down Expand Up @@ -1538,8 +1540,14 @@ def __init__(self, code, prefix=""):
@property
def error_string(self):
"""Raw libsndfile error message."""
err_str = _snd.sf_error_number(self.code)
return _ffi.string(err_str).decode('utf-8', 'replace')
if self.code:
err_str = _snd.sf_error_number(self.code)
return _ffi.string(err_str).decode('utf-8', 'replace')
else:
# Due to race conditions, if used concurrently, sf_error() may
# return 0 (= no error) even if an error has happened.
# See https://github.com/erikd/libsndfile/issues/610 for details.
return "(Garbled error message from libsndfile)"

def __str__(self):
return self.prefix + self.error_string
24 changes: 24 additions & 0 deletions tests/test_pysoundfile.py
Expand Up @@ -8,6 +8,7 @@
import sys
import gc
import weakref
import threading

# floating point data is typically limited to the interval [-1.0, 1.0],
# but smaller/larger values are supported as well
Expand Down Expand Up @@ -750,6 +751,29 @@ def test_read_into_out_over_end_with_fill_should_return_full_data_and_write_into
assert np.all(data[2:] == 0)
assert out.shape == (4, sf_stereo_r.channels)

def test_concurren_open_error_reporting(file_inmemory):
# Test that no sf_open errors are missed when pysoundfile is used
# concurrently (there are race conditions in libsndfile's error reporting).

n_threads = 4
n_trials_per_thread = 10

n_reported_errors = [0]

def target():
for _ in range(n_trials_per_thread):
try:
sf.SoundFile(file_inmemory)
except sf.LibsndfileError:
n_reported_errors[0] += 1

threads = [threading.Thread(target=target) for _ in range(n_threads)]
for thread in threads:
thread.start()
for thread in threads:
thread.join()
assert n_reported_errors[0] == n_threads * n_trials_per_thread


# -----------------------------------------------------------------------------
# Test buffer read
Expand Down