joeyutong commented on code in PR #944:
URL: https://github.com/apache/flink-agents/pull/944#discussion_r3703612061
##########
runtime/src/main/java/org/apache/flink/agents/runtime/python/utils/PythonActionExecutor.java:
##########
@@ -188,17 +189,32 @@ public boolean callPythonAwaitable(String
pythonAwaitableRef) {
}
public void close() throws Exception {
- if (interpreter != null) {
- if (pythonAsyncThreadPool != null) {
- interpreter.invoke(CLOSE_ASYNC_THREAD_POOL,
pythonAsyncThreadPool);
- }
+ PyObject asyncThreadPool = pythonAsyncThreadPool;
+ PyObject runnerContext = pythonRunnerContext;
+ pythonAsyncThreadPool = null;
+ pythonRunnerContext = null;
+
+ Exception exception = null;
+ try {
+ closePythonObject(CLOSE_ASYNC_THREAD_POOL, asyncThreadPool);
+ } catch (Exception e) {
+ exception = ExceptionUtils.firstOrSuppressed(e, exception);
+ }
+ try {
+ closePythonObject(CLOSE_FLINK_RUNNER_CONTEXT, runnerContext);
+ } catch (Exception e) {
+ exception = ExceptionUtils.firstOrSuppressed(e, exception);
+ }
+
+ if (exception != null) {
+ throw exception;
+ }
+ }
- if (pythonRunnerContext != null) {
- try {
- interpreter.invoke(CLOSE_FLINK_RUNNER_CONTEXT,
pythonRunnerContext);
- } finally {
- pythonRunnerContext = null;
- }
+ private void closePythonObject(String closeFunction, PyObject
pythonObject) throws Exception {
+ if (pythonObject != null) {
+ try (pythonObject) {
Review Comment:
Good catch. Both handles are live, and the A/B heap dumps confirmed the same
retention shape, so I widened this PR instead of opening a follow-up:
- `PythonResourceAdapterImpl` now owns and closes `pythonResourceContext`;
nulling the field first makes repeated close safe.
- `Mem0LongTermMemory` now runs Python-level cleanup and always closes
`pyMem0`, including when logical cleanup fails; it is also idempotent.
- `PythonBridgeManager` now closes Mem0 and the resource adapter before the
interpreter/environment, while continuing all closes after failures. This also
covers Mem0-initialized subtasks that never created a Java `RunnerContext`.
- `FlinkRunnerContext.close()` clears `__ltm` before cleanup and still
closes the resource cache if LTM cleanup fails. This breaks the Python
context/Mem0 cycle so releasing the Pemja handles can collapse the full Java ->
Python -> Java retention chain.
- Added lifecycle, failure-path, and repeated-close tests. In focused
20-restart HPROF A/B runs, each sibling baseline retained 84 target `PyObject`
handles after termination; the native-close variants retained 0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
