This is an automated email from the ASF dual-hosted git repository.
raulcd pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 9ec70551b9 GH-50684: [Python][FlightRPC] Break the reference cycle
between the C++ FlightServerBase and the Python object to avoid leaking server
(#50687)
9ec70551b9 is described below
commit 9ec70551b9caa042f15c475e1fe0a611bb7002ba
Author: Raúl Cumplido <[email protected]>
AuthorDate: Fri Jul 31 08:59:19 2026 +0200
GH-50684: [Python][FlightRPC] Break the reference cycle between the C++
FlightServerBase and the Python object to avoid leaking server (#50687)
### Rationale for this change
PyFlightServer keeps a reference towards the Python server via
`OwnedRefNoGIL server_`, the Python server also keeps a reference of the C++
`PyFlightServer` creating a cycle that is never freed during the process
lifetime.
### What changes are included in this PR?
Create a new `ReleasePythonServerRef` method that is called after any
`server.Shutdown` (including at `__exit__`) allowing for the `OwnedRefNoGIL` to
be cleared breaking the cycle. This lets normal reference counting free the
previously leaked Python object.
### Are these changes tested?
Yes, the newly added tests were failing leaking the references before the
fix.
Currently a test demonstrating a leak when not calling server.Shutdown is
added for discussion purposes.
### Are there any user-facing changes?
No
* GitHub Issue: #50684
Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
---
python/pyarrow/_flight.pyx | 4 +--
python/pyarrow/includes/libarrow_flight.pxd | 1 +
python/pyarrow/src/arrow/python/flight.cc | 6 +++++
python/pyarrow/src/arrow/python/flight.h | 3 +++
python/pyarrow/tests/test_flight.py | 42 +++++++++++++++++++++++++++--
5 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/python/pyarrow/_flight.pyx b/python/pyarrow/_flight.pyx
index 82cbf87f51..f8118fbf61 100644
--- a/python/pyarrow/_flight.pyx
+++ b/python/pyarrow/_flight.pyx
@@ -2883,10 +2883,9 @@ cdef class _FlightServerFinalizer(_Weakrefable):
try:
with nogil:
status = server.Shutdown()
- if status.ok():
- status = server.Wait()
check_flight_status(status)
finally:
+ server.ReleasePythonServerRef()
self.server.reset()
@@ -3234,6 +3233,7 @@ cdef class FlightServerBase(_Weakrefable):
raise ValueError("shutdown() on uninitialized FlightServerBase")
with nogil:
check_flight_status(self.server.get().Shutdown())
+ self.server.get().ReleasePythonServerRef()
def wait(self):
"""Block until server is terminated with shutdown."""
diff --git a/python/pyarrow/includes/libarrow_flight.pxd
b/python/pyarrow/includes/libarrow_flight.pxd
index a89137b845..a88cfa6664 100644
--- a/python/pyarrow/includes/libarrow_flight.pxd
+++ b/python/pyarrow/includes/libarrow_flight.pxd
@@ -536,6 +536,7 @@ cdef extern from "arrow/python/flight.h" namespace
"arrow::py::flight" nogil:
CStatus ServeWithSignals() except *
CStatus Shutdown()
CStatus Wait()
+ void ReleasePythonServerRef()
cdef cppclass PyServerAuthHandler\
" arrow::py::flight::PyServerAuthHandler"(CServerAuthHandler):
diff --git a/python/pyarrow/src/arrow/python/flight.cc
b/python/pyarrow/src/arrow/python/flight.cc
index 5ef8a1dd6b..db868e8826 100644
--- a/python/pyarrow/src/arrow/python/flight.cc
+++ b/python/pyarrow/src/arrow/python/flight.cc
@@ -86,6 +86,12 @@ PyFlightServer::PyFlightServer(PyObject* server, const
PyFlightServerVtable& vta
server_.reset(server);
}
+void PyFlightServer::ReleasePythonServerRef() {
+ // Resets OwnedRefNoGIL to break the reference cycle between the C++
FlightServerBase
+ // and the Python object.
+ server_.reset();
+}
+
Status PyFlightServer::ListFlights(
const arrow::flight::ServerCallContext& context,
const arrow::flight::Criteria* criteria,
diff --git a/python/pyarrow/src/arrow/python/flight.h
b/python/pyarrow/src/arrow/python/flight.h
index 8a1f4c750a..283e2c26cc 100644
--- a/python/pyarrow/src/arrow/python/flight.h
+++ b/python/pyarrow/src/arrow/python/flight.h
@@ -171,6 +171,9 @@ class ARROW_PYFLIGHT_EXPORT PyFlightServer : public
arrow::flight::FlightServerB
Status ListActions(const arrow::flight::ServerCallContext& context,
std::vector<arrow::flight::ActionType>* actions) override;
+ // Breaks the reference cycle between the C++ FlightServerBase and the
Python object.
+ void ReleasePythonServerRef();
+
private:
OwnedRefNoGIL server_;
PyFlightServerVtable vtable_;
diff --git a/python/pyarrow/tests/test_flight.py
b/python/pyarrow/tests/test_flight.py
index a66be6186d..e1c404d81e 100644
--- a/python/pyarrow/tests/test_flight.py
+++ b/python/pyarrow/tests/test_flight.py
@@ -17,7 +17,10 @@
import ast
import base64
+from datetime import datetime
+import gc
import itertools
+import json
import os
import pathlib
import signal
@@ -28,8 +31,7 @@ import tempfile
import threading
import time
import traceback
-import json
-from datetime import datetime
+import weakref
try:
import numpy as np
@@ -1128,6 +1130,42 @@ def test_flight_server_location_argument():
assert isinstance(server, FlightServerBase)
+# The following tests are for GH-50684, which was a memory leak
+# in FlightServerBase.
+def test_flight_server_is_freed():
+ # Calling server.shutdown manually should free the server object.
+ server = FlightServerBase('grpc://localhost:0')
+ server.shutdown()
+ ref = weakref.ref(server)
+ del server
+ gc.collect()
+ assert ref() is None
+
+
+def test_flight_server_is_freed_on_exit():
+ # Using FlightServerBase as a context manager should free
+ # the server object on exit.
+ with FlightServerBase('grpc://localhost:0') as server:
+ ref = weakref.ref(server)
+ del server
+ gc.collect()
+ assert ref() is None
+
+
[email protected](
+ reason="GH-50684: FlightServerBase is not freed on delete without shutdown"
+)
+def test_flight_server_is_freed_without_shutdown():
+ # GH-50684: Not calling server.shutdown() currently leaks the server
object.
+ # This test is expected to fail until the issue is fixed but is included
+ # for completeness and further discussion.
+ server = FlightServerBase('grpc://localhost:0')
+ ref = weakref.ref(server)
+ del server
+ gc.collect()
+ assert ref() is None
+
+
def test_server_exit_reraises_exception():
with pytest.raises(ValueError):
with FlightServerBase():
