make sure the GIL is not held when interacting with the PyFileSystem

duckdb · Sep 5, 2024 · 24abb93 · 24abb93
1 parent dffc4ff
commit 24abb93
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 0 deletions.
diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp
@@ -874,6 +874,7 @@ unique_ptr<DuckDBPyRelation> DuckDBPyConnection::ReadJSON(
 		auto_detect = true;
 	}
 
+	py::gil_scoped_release gil;
 	auto read_json_relation =
 	    make_shared_ptr<ReadJSONRelation>(connection.context, name, std::move(options), auto_detect);
 	if (read_json_relation == nullptr) {
@@ -1383,6 +1384,7 @@ unique_ptr<DuckDBPyRelation> DuckDBPyConnection::ReadCSV(const py::object &name_
 
 	// Create the ReadCSV Relation using the 'options'
 
+	py::gil_scoped_release gil;
 	auto read_csv_p = connection.ReadCSV(name, std::move(bind_parameters));
 	auto &read_csv = read_csv_p->Cast<ReadCSVRelation>();
 	if (file_like_object_wrapper) {
@@ -1548,6 +1550,7 @@ unique_ptr<DuckDBPyRelation> DuckDBPyConnection::FromParquet(const string &file_
 		}
 		named_parameters["compression"] = Value(py::str(compression));
 	}
+	py::gil_scoped_release gil;
 	return make_uniq<DuckDBPyRelation>(connection.TableFunction("parquet_scan", params, named_parameters)->Alias(name));
 }
 

diff --git a/tools/pythonpkg/src/pyfilesystem.cpp b/tools/pythonpkg/src/pyfilesystem.cpp
@@ -146,12 +146,14 @@ string PythonFilesystem::PathSeparator(const string &path) {
 	return "/";
 }
 int64_t PythonFilesystem::GetFileSize(FileHandle &handle) {
+	D_ASSERT(!py::gil_check());
 	// TODO: this value should be cached on the PythonFileHandle
 	PythonGILWrapper gil;
 
 	return py::int_(filesystem.attr("size")(handle.path));
 }
 void PythonFilesystem::Seek(duckdb::FileHandle &handle, uint64_t location) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	auto seek = PythonFileHandle::GetHandle(handle).attr("seek");
@@ -170,18 +172,21 @@ bool PythonFilesystem::CanHandleFile(const string &fpath) {
 	return false;
 }
 void PythonFilesystem::MoveFile(const string &source, const string &dest, optional_ptr<FileOpener> opener) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	auto move = filesystem.attr("mv");
 	move(py::str(source), py::str(dest));
 }
 void PythonFilesystem::RemoveFile(const string &filename, optional_ptr<FileOpener> opener) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	auto remove = filesystem.attr("rm");
 	remove(py::str(filename));
 }
 time_t PythonFilesystem::GetLastModifiedTime(FileHandle &handle) {
+	D_ASSERT(!py::gil_check());
 	// TODO: this value should be cached on the PythonFileHandle
 	PythonGILWrapper gil;
 
@@ -190,6 +195,7 @@ time_t PythonFilesystem::GetLastModifiedTime(FileHandle &handle) {
 	return py::int_(last_mod.attr("timestamp")());
 }
 void PythonFilesystem::FileSync(FileHandle &handle) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	PythonFileHandle::GetHandle(handle).attr("flush")();
@@ -198,11 +204,13 @@ bool PythonFilesystem::DirectoryExists(const string &directory, optional_ptr<Fil
 	return Exists(directory, "isdir");
 }
 void PythonFilesystem::RemoveDirectory(const string &directory, optional_ptr<FileOpener> opener) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	filesystem.attr("rm")(directory, py::arg("recursive") = true);
 }
 void PythonFilesystem::CreateDirectory(const string &directory, optional_ptr<FileOpener> opener) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	filesystem.attr("mkdir")(py::str(directory));
@@ -211,6 +219,7 @@ bool PythonFilesystem::ListFiles(const string &directory, const std::function<vo
                                  FileOpener *opener) {
 	static py::str DIRECTORY("directory");
 
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 	bool nonempty = false;
 
@@ -223,6 +232,7 @@ bool PythonFilesystem::ListFiles(const string &directory, const std::function<vo
 	return nonempty;
 }
 void PythonFilesystem::Truncate(FileHandle &handle, int64_t new_size) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	filesystem.attr("touch")(handle.path, py::arg("truncate") = true);
@@ -231,6 +241,7 @@ bool PythonFilesystem::IsPipe(const string &filename, optional_ptr<FileOpener> o
 	return false;
 }
 idx_t PythonFilesystem::SeekPosition(FileHandle &handle) {
+	D_ASSERT(!py::gil_check());
 	PythonGILWrapper gil;
 
 	return py::int_(PythonFileHandle::GetHandle(handle).attr("tell")());