claudevdm commented on code in PR #40822:
URL: https://github.com/apache/beam/pull/40822#discussion_r4389392627
##########
sdks/python/apache_beam/utils/multi_process_shared.py:
##########
@@ -150,8 +152,22 @@ def release(self, proxy):
def unsafe_hard_delete(self):
with self.lock:
if self.initialied:
+ obj = self.obj
del self.obj
self.initialied = False
+ # When hard-deleting in a spawned server process that exits via
+ # os._exit(0), an in-flight RPC thread (e.g. a timed-out call) may
+ # still hold a stack reference to `obj`, preventing CPython from
+ # invoking `obj.__del__()` before `os._exit(0)`.
+ if (self._hard_delete_callback is not None and
+ sys.getrefcount(obj) > 2 and hasattr(type(obj), '__del__')):
+ try:
+ type(obj).__del__(obj)
Review Comment:
Could this hang for a model handler with a lock on the model +
with_exception_handling(timeout ?
1. The worker calls predict.
2. It runs in the model process, takes the model's lock and hangs.
3. The timeout fires in the worker.
4. The worker stops waiting and schedules the model for deletion. Predict is
still hung and still holding the lock.
5. The worker loads a fresh copy of the model and carries on.
6. At least 2× the timeout later, finish_bundle sends "hard delete" to the
old model process. Forget the model, notice predict still holds it, and call
the model's cleanup first. The cleanup waits for the lock, which predict never
releases, so os._exit never runs and finish_bundle waits forever for a reply.
Potentially run the cleanup on a separate thread and wait for it only up to
a timeout, then exit regardless, so a cleanup stuck behind the hung call can't
block the hard delete?
##########
sdks/python/apache_beam/utils/multi_process_shared.py:
##########
@@ -150,8 +152,22 @@ def release(self, proxy):
def unsafe_hard_delete(self):
with self.lock:
if self.initialied:
+ obj = self.obj
del self.obj
self.initialied = False
+ # When hard-deleting in a spawned server process that exits via
+ # os._exit(0), an in-flight RPC thread (e.g. a timed-out call) may
+ # still hold a stack reference to `obj`, preventing CPython from
+ # invoking `obj.__del__()` before `os._exit(0)`.
+ if (self._hard_delete_callback is not None and
+ sys.getrefcount(obj) > 2 and hasattr(type(obj), '__del__')):
+ try:
+ type(obj).__del__(obj)
Review Comment:
The model's cleanup potentially runs twice with
`with_exception_handling(timeout)` if the hung call finishes while the cleanup
is still running?
1. A call on the model hangs. The timeout fires, and the worker schedules
the model for deletion. The call is still running in the model process.
2. Later, the hard delete sees the call still holds the model
(`sys.getrefcount(obj) > 2`), so it calls `type(obj).__del__(obj)` (run 1).
3. The cleanup makes the hung call end, e.g. by killing a subprocess the
call was polling. Once the call ends and its worker thread exits, nothing else
holds the model.
4. Run 1 returns and `del obj` drops the last reference. Python frees the
model and runs `__del__` again (run 2), because it doesn't know about run 1.
A cleanup that isn't safe to repeat would then run twice. Potentially keep
the model referenced until exit after running the cleanup (e.g. append it to a
module-level list), so `__del__` can't run a second time?
##########
sdks/python/apache_beam/utils/multi_process_shared.py:
##########
@@ -150,8 +152,22 @@ def release(self, proxy):
def unsafe_hard_delete(self):
with self.lock:
if self.initialied:
+ obj = self.obj
del self.obj
self.initialied = False
+ # When hard-deleting in a spawned server process that exits via
+ # os._exit(0), an in-flight RPC thread (e.g. a timed-out call) may
+ # still hold a stack reference to `obj`, preventing CPython from
+ # invoking `obj.__del__()` before `os._exit(0)`.
+ if (self._hard_delete_callback is not None and
+ sys.getrefcount(obj) > 2 and hasattr(type(obj), '__del__')):
Review Comment:
Potentially check for other holders with a weakref instead (drop our
reference, then see whether `ref()` still returns the object)?
--
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]
