Skip to content

Commit

Permalink
Merge pull request #11980 from Tishj/pyfs_needs_gil
Browse files Browse the repository at this point in the history
[Python] Grab the GIL in the destructor of PyFilesystem
  • Loading branch information
Mytherin committed May 13, 2024
2 parents ffb3f97 + 1c17c6d commit e9e1992
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
3 changes: 2 additions & 1 deletion tools/pythonpkg/src/include/duckdb_python/pyfilesystem.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,15 @@ class PythonFileHandle : public FileHandle {
class PythonFilesystem : public FileSystem {
private:
const vector<string> protocols;
const AbstractFileSystem filesystem;
AbstractFileSystem filesystem;
std::string DecodeFlags(FileOpenFlags flags);
bool Exists(const string &filename, const char *func_name) const;

public:
explicit PythonFilesystem(vector<string> protocols, AbstractFileSystem filesystem)
: protocols(std::move(protocols)), filesystem(std::move(filesystem)) {
}
~PythonFilesystem() override;

protected:
string GetName() const override {
Expand Down
9 changes: 9 additions & 0 deletions tools/pythonpkg/src/pyfilesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ PythonFileHandle::~PythonFileHandle() {
}
}

PythonFilesystem::~PythonFilesystem() {
try {
PythonGILWrapper gil;
filesystem.dec_ref();
filesystem.release();
} catch (...) { // NOLINT
}
}

string PythonFilesystem::DecodeFlags(FileOpenFlags flags) {
// see https://stackoverflow.com/a/58925279 for truth table of python file modes
bool read = flags.OpenForReading();
Expand Down

0 comments on commit e9e1992

Please sign in to comment.