Skip to content

Commit

Permalink
python: fix circular reference in auto-bounded wrapper methods
Browse files Browse the repository at this point in the history
When the user of Python objects inheriting from `Wrapper` attempt to
access an unkonwn method (i.e., a method that is not manually written
and included in the bindings)will automatically search the cffi
auto-generated bindings for functions with a matching name. Once a match
is found, the function is bound as a method to the object to effectively
cache the search and make future accesses quicker.  This bound method
creates a circular reference between the `Wrapper` and it's method.  In
the case of `Future`s, this resulted in a leak and the piling up of open
futures, as documented in flux-framework#2566.

To avoid this circular reference, use the `weakref` library to create a
weak reference to the object when constructing the bound method.  When
the object leaves scope, it is now immediately destructed.

Remove the no longer necessary calls to `future._clear()`

Closes flux-framework#2566.
  • Loading branch information
SteVwonder committed Dec 10, 2019
1 parent c5f1175 commit 64f22e3
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 5 deletions.
4 changes: 0 additions & 4 deletions src/bindings/python/flux/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ def submit_get_id(future):
def submit(flux_handle, jobspec, priority=lib.FLUX_JOB_PRIORITY_DEFAULT, flags=0):
future = submit_async(flux_handle, jobspec, priority, flags)
jid = submit_get_id(future)
# pylint: disable=protected-access
future.pimpl._clear()
return jid


Expand Down Expand Up @@ -82,6 +80,4 @@ def wait_get_status(future):
def wait(flux_handle, jobid=lib.FLUX_JOBID_ANY):
future = wait_async(flux_handle, jobid)
status = wait_get_status(future)
# pylint: disable=protected-access
future.pimpl._clear()
return status
4 changes: 3 additions & 1 deletion src/bindings/python/flux/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import os
import errno
import inspect
import weakref

import six


Expand Down Expand Up @@ -319,7 +321,7 @@ def __getattr__(self, name):
return fun

new_fun = self.check_wrap(fun, name)
new_method = six.create_bound_method(new_fun, self)
new_method = six.create_bound_method(new_fun, weakref.proxy(self))

# Store the wrapper function into the instance
# to prevent a second lookup
Expand Down

0 comments on commit 64f22e3

Please sign in to comment.