python: ReferenceError: weakly-referenced object no longer exists #2671
Comments
|
😱 That is absolutely horrifying. I suspect the weakref that I inserted to fix #2549 is somehow implicated in this. I'll stick this on my TODO list, but I probably won't be able to look at it in earnest until Monday 😞 |
|
Hitting this with a similar script when python rpc gets an error (e.g. EPERM) |
|
Thanks @grondo for the reproducer! I took a poke at this, and I think now I get what is happening but not necessarily how to solve it. Here is my best summary of the situation:
My hypothesis for why reference inside the I'm scrapping the bottom of the barrel for ideas as to how to solve this or debug further. We can avoid using For reference on how I got to these conclusions: diff --git a/src/bindings/python/flux/core/handle.py b/src/bindings/python/flux/core/handle.py
index 44810831b..96bba97aa 100644
--- a/src/bindings/python/flux/core/handle.py
+++ b/src/bindings/python/flux/core/handle.py
@@ -36,7 +36,11 @@ class Flux(Wrapper):
prefixes=["flux_", "FLUX_"],
destructor=raw.flux_close,
)
-
+ #self.destructor = lib.flux_close
+ print(
+ "Creating new Flux instance, with destructor {}. raw.flux_close == {}".format(
+ self.destructor, raw.flux_close)
+ )
if handle is None:
try:
self.handle = raw.flux_open(url, flags)
diff --git a/src/bindings/python/flux/core/inner.py b/src/bindings/python/flux/core/inner.py
index e729f7842..ab1fcdf0f 100644
--- a/src/bindings/python/flux/core/inner.py
+++ b/src/bindings/python/flux/core/inner.py
@@ -21,7 +21,6 @@ class Core(Wrapper):
"""Set up the wrapper interface for functions prefixed with flux_"""
super(Core, self).__init__(ffi, lib, prefixes=["flux_", "FLUX_"])
-
# keeping this for compatibility
# pylint: disable=invalid-name
raw = Core()
diff --git a/src/bindings/python/flux/wrapper.py b/src/bindings/python/flux/wrapper.py
index d981c2a87..46ad25a31 100644
--- a/src/bindings/python/flux/wrapper.py
+++ b/src/bindings/python/flux/wrapper.py
@@ -337,6 +337,7 @@ class Wrapper(WrapperBase):
return
if self.destructor is None:
return
+ print("Calling destructor ({}) on {}".format(self.destructor, self))
self.destructor(handle)
# ensure we don't double destruct
self._handle = None
@@ -361,6 +362,7 @@ class Wrapper(WrapperBase):
self._handle = h
def __del__(self):
+ print("Calling __del__ on {}".format(self))
self._clear()
def __enter__(self):That produces the following output: |
|
Oof... ok, this one might take some tweaking... I think I might have a solution though, short of going to the generated eplicit bindings option. What we should have been doing is adding the generated method to the class not the instance anyway, but not to Wrapper, specifically to the derived class that is being used by the object new_method = six.create_bound_method(new_fun, weakref.proxy(self))
# Store the wrapper function into the instance
# to prevent a second lookup
setattr(self, name, new_method)with something kinda like this: # no method conversion needed, functions in classes are methods
# new_method = six.create_bound_method(new_fun, weakref.proxy(self))
# Store the wrapper function into the class
# to prevent a second lookup
setattr(type(self), name, new_fun)Also we can't return the thing directly anymore, we need to wrap it and do the bound method thing for the return value here to get the correct behavior, but that should be reaped immediately and not hang around to cause us grief. Something like what we used to have: return types.MethodType(new_fun, self)It should get rid of the weakref entirely, give us per-derived-class method lookups and hopefully fix up some other issues. |
|
Thanks @trws for taking a look at this! I tried out your suggested changes. It looks like the binding of the method to the class doesn't work without further modification due to RPC and Future sharing the same InnerWrapper class, whose instances match entirely different sets of function prefixes (i.e., After walking that change back, it looks like the explicit binding of the method is still required too. Without the method explicitly being bound, the So walking those two changes back, and I think we are back where we were in #2549, minus the need for |
Rather than caching in the instance, this should remove dangling reference issues like in flux-framework#2671, and hopefully also be somewhat faster in that the lookup happens once per *type* rather than once per *instance*.
Rather than caching in the instance, this should remove dangling reference issues like in flux-framework#2671, and hopefully also be somewhat faster in that the lookup happens once per *type* rather than once per *instance*.
Rather than caching in the instance, this should remove dangling reference issues like in flux-framework#2671, and hopefully also be somewhat faster in that the lookup happens once per *type* rather than once per *instance*.
|
I believe this was fixed with the merging of #2878. We can re-open if not. |
I'm seeing this very strange error with Python 3.6.9 on an Ubuntu system. I'm really baffled by this behavior, but hopefully it is easily explained.
I've boiled the reproducer down to the following script:
This script succeeds, but prints a warning and backtrace
The baffling thing is if I comment out the dummy function that wasn't even called, I don't get a warning:
This occurs for me whenever there is a global function defined in the script, whether my code calls it or not.
Anyone know what is going on here?
The text was updated successfully, but these errors were encountered: