Skip to content

Commit ab36dbb

Browse files
authored
Fix for reading certain Ogg files on Linux via file-like objects. (#436)
* Fix for reading certain Ogg files on Linux via file-like objects. * Fix type hints in test.
1 parent 83cb3dd commit ab36dbb

File tree

4 files changed

+96
-1
lines changed

4 files changed

+96
-1
lines changed

pedalboard/io/PythonFileLike.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#pragma once
1919

20+
#include <cerrno>
2021
#include <mutex>
2122
#include <optional>
2223

@@ -45,6 +46,31 @@ inline void raise() {
4546
}
4647
}; // namespace PythonException
4748

49+
/**
50+
* @brief A helper class that clears the thread's errno when destructed.
51+
*
52+
* This is used to avoid failure when the following sequence of events occurs:
53+
*
54+
* 1. A Python file-like object is passed to Pedalboard, which could call any
55+
* other native code in its methods
56+
* 2. Pedalboard calls a Python file-like object's methods (e.g. read(), seek(),
57+
* tell(), seekable())
58+
* 3. The native code sets errno to a non-zero value, but does not clear it
59+
* 4. Pedalboard's codecs (Ogg Vorbis, etc) check errno and fail to decode
60+
* 5. The user is presented with a cryptic error message
61+
*
62+
* This makes it seem like we're ignoring errno, but we're not; the Python-level
63+
* code should throw an exception if the file-like object has an error, and we
64+
* handle errors at that level correctly.
65+
*
66+
* See also: https://en.wikipedia.org/wiki/Errno.h
67+
*/
68+
class ClearErrnoBeforeReturn {
69+
public:
70+
ClearErrnoBeforeReturn() {}
71+
~ClearErrnoBeforeReturn() { errno = 0; }
72+
};
73+
4874
/**
4975
* A tiny helper class that downgrades a write lock
5076
* to a read lock for the given scope. If we can't

pedalboard/io/PythonInputStream.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ class PythonInputStream : public juce::InputStream, public PythonFileLike {
6363

6464
juce::int64 getTotalLength() noexcept override {
6565
ScopedDowngradeToReadLockWithGIL lock(objectLock);
66+
ClearErrnoBeforeReturn clearErrnoBeforeReturn;
6667
py::gil_scoped_acquire acquire;
6768

6869
if (PythonException::isPending())
@@ -99,6 +100,7 @@ class PythonInputStream : public juce::InputStream, public PythonFileLike {
99100
// sign that something is broken!
100101
jassert(buffer != nullptr && bytesToRead >= 0);
101102
ScopedDowngradeToReadLockWithGIL lock(objectLock);
103+
ClearErrnoBeforeReturn clearErrnoBeforeReturn;
102104

103105
py::gil_scoped_acquire acquire;
104106
if (PythonException::isPending())
@@ -161,6 +163,7 @@ class PythonInputStream : public juce::InputStream, public PythonFileLike {
161163
juce::int64 totalLength = getTotalLength();
162164

163165
ScopedDowngradeToReadLockWithGIL lock(objectLock);
166+
ClearErrnoBeforeReturn clearErrnoBeforeReturn;
164167
py::gil_scoped_acquire acquire;
165168

166169
if (PythonException::isPending())
@@ -183,6 +186,7 @@ class PythonInputStream : public juce::InputStream, public PythonFileLike {
183186

184187
juce::int64 getPosition() noexcept override {
185188
ScopedDowngradeToReadLockWithGIL lock(objectLock);
189+
ClearErrnoBeforeReturn clearErrnoBeforeReturn;
186190
py::gil_scoped_acquire acquire;
187191

188192
if (PythonException::isPending())
@@ -201,6 +205,7 @@ class PythonInputStream : public juce::InputStream, public PythonFileLike {
201205

202206
bool setPosition(juce::int64 pos) noexcept override {
203207
ScopedDowngradeToReadLockWithGIL lock(objectLock);
208+
ClearErrnoBeforeReturn clearErrnoBeforeReturn;
204209
py::gil_scoped_acquire acquire;
205210

206211
if (PythonException::isPending())

pedalboard/io/ReadableAudioFile.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class ReadableAudioFile
155155
} else {
156156
ss << " has its stream position set to the end of the stream ("
157157
<< originalStreamPosition;
158-
ss << "bytes).";
158+
ss << " bytes).";
159159
}
160160
ss << " Try seeking this file-like object back to its start before "
161161
"passing it to AudioFile";

tests/test_io.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,3 +1592,67 @@ def test_write_two_by_two_buffer_with_hint():
15921592
f.write(np.zeros((2, 2), dtype=np.float32))
15931593

15941594
assert f.tell() == 2
1595+
1596+
1597+
class ErrnoTriggeringFileLike(BinaryIO):
1598+
"""A file-like object that triggers errno to be set through real failed operations."""
1599+
1600+
def __init__(self, wrapped_file):
1601+
self._file = wrapped_file
1602+
# Create a path that definitely doesn't exist for triggering ENOENT
1603+
self._nonexistent_path = str(pathlib.Path("definitely_does_not_exist_12345.txt"))
1604+
1605+
def _trigger_errno(self):
1606+
# Try to open a non-existent file, which sets errno to ENOENT (2)
1607+
# We catch the exception but errno remains set
1608+
try:
1609+
open(self._nonexistent_path, "rb")
1610+
except FileNotFoundError:
1611+
pass # Expected - errno is now set to ENOENT
1612+
1613+
def read(self, *args, **kwargs):
1614+
result = self._file.read(*args, **kwargs)
1615+
self._trigger_errno()
1616+
return result
1617+
1618+
def seek(self, *args, **kwargs):
1619+
result = self._file.seek(*args, **kwargs)
1620+
self._trigger_errno()
1621+
return result
1622+
1623+
def tell(self):
1624+
result = self._file.tell()
1625+
self._trigger_errno()
1626+
return result
1627+
1628+
def seekable(self):
1629+
result = self._file.seekable()
1630+
self._trigger_errno()
1631+
return result
1632+
1633+
1634+
@pytest.mark.parametrize("format_name", ["wav", "ogg"])
1635+
def test_errno_cleared_after_python_file_operations(format_name: str):
1636+
"""
1637+
Regression test to ensure that errno values set by Python file-like objects
1638+
don't leak through and cause codec failures. Before the fix (ClearErrnoBeforeReturn),
1639+
errno values set during Python file operations would cause mysterious failures in
1640+
codecs like Ogg Vorbis that check errno.
1641+
1642+
This test creates a file-like wrapper that intentionally sets errno to a non-zero
1643+
value after each operation, simulating what might happen with certain third-party
1644+
libraries or file systems. The fix ensures these errno values are cleared before
1645+
returning to codec code.
1646+
"""
1647+
1648+
buf = io.BytesIO()
1649+
buf.name = f"test.{format_name}"
1650+
with pedalboard.io.AudioFile(buf, "w", 44100, 2, format=format_name) as af:
1651+
af.write(np.random.rand(2, 44100))
1652+
1653+
# Add some random garbage past the end of the file to trigger the bug:
1654+
buf.write(b"\x00\x00")
1655+
buf.seek(0)
1656+
1657+
with pedalboard.io.AudioFile(ErrnoTriggeringFileLike(buf)) as af:
1658+
assert af.samplerate == 44100

0 commit comments

Comments
 (0)