Reflect.construct not producing expected results about pyodide HOT 8 CLOSED

I am not sure I should bring ffi in to make it work (other runtimes don't need it)

Well if you want code that is runtime agnostic, you could manage this with:

try:
   from pyodide.ffi import create_once_callable
except ImportError:
   create_once_callable = lambda x: x

# ... code using create_once_callable

The same trick works well for create_proxy too.

from pyodide.

P.S. in an ideal world I'd love to also be able to write Reflect.construct(Promise, [callback]) and use that list as transparent way to signal a well known JS array ... but I guess that's rather a feature request or a "nice to have" thing ... non-usable Reflect in a JS way is definitively a bigger issue.

from pyodide.

If you use the following:

from pyodide.ffi import create_once_callable
p = Reflect.construct(Promise, Array(create_once_callable(trap)))

it works as expected. It also works fine to use:

Promise.new(trap)

or

from pyodide.ffi import create_once_callable
p = Reflect.construct(Promise, [trap])

If you await the promise in your original example, you get a clearer (but still slightly confusing) error message:

pyodide.runPythonAsync(`
        from js import Array, Promise, Reflect
        def trap(res, rej):
          print(1)
          res(3)

        print(0)
        p = Reflect.construct(Promise, Array(trap))
        print(2)
        await p.then(print)
`);

pyodide.ffi.JsException: Error: This borrowed proxy was automatically destroyed at the end of a function call. Try using create_proxy or create_once_callable.
The object was of type "function" and had repr "<function trap at 0x1027288>"

from this error, you might figure out that the issue is resource management around the Array(trap) call, but it is a bit obscure. I think it might help to add the name of the function that was called to the error context.

in an ideal world I'd love to also be able to write Reflect.construct(Promise, [callback]) and use that list as transparent way to signal a well known JS array

Can you explain what you mean by this? I do not understand.

from pyodide.

Ah so

p = Reflect.construct(Promise, [trap])

fails in v0.23.2 but the failure is fixed on the main branch probably due to changes made to resolve some of your other bug reports @WebReflection.

from pyodide.

In particular, it works to pass a python list since #3853 was merged.

from pyodide.

so ... is this a "fixed and ready to land" bogus?

from pyodide.

btw, my Array(trap) was after the fact I am not sure I should bring ffi in to make it work (other runtimes don't need it) but I am OK with this resolved as won't fix, specially knowing Reflect.construct(Promise, [fn]) works ... I wonder why the latter is straight forward but the former isn't though, but then again none of these are blockers to me, just a documentation matter.

from pyodide.

Since you asked, here is a long explanation about why it works this way. I think this is the "best possible" design, but it is more designed around the constraint of making it possible and ergonomic to use Pyodide in cases things where memory management matters at the cost of making it slightly less ergonomic in the case where it doesn't matter.

Please let me know if you have questions or opinions!

The issue is about garbage collection and resource management. Basically, the trouble is that to convert a Python object to a JavaScript PyProxy object, the JavaScript PyProxy object owns a reference to the Python object and keeps it alive. If you leak the PyProxy, then you have to wait until the v8 FinalizationRegistry decides to run the finalizer. In practice, this often takes a long time. Furthermore, there is no way to advice v8 about how much memory is actually owned by a proxy.

For instance, imagine that the object owns a slice of the wasm heap. We can imagine the following code:

const finalizationRegistry = new FinalizationRegistry(ptr => {
   // Release memory from heap
   Module._free(ptr);
});
const size = 100000;
const ptr = Module._malloc(size);
const o = {
   ptr,
   size,
   // Fancy methods to do stuff with the ptr
}
// No way to inform finalizationRegistry that ptr owns 100,000 bytes of 
// memory and would be useful to free soon, browser garbage collector
// only sees that it has two Number fields.
finalizationRegistry.register(o, ptr, o);

There's no way around this when calling into Python, but if we are calling out from Python into JavaScript then we can create the object that owns the memory before the function call and automatically release it when the function call is done.
Most functions don't store references to their arguments, so all of these existing js functions work without change and don't cause any memory leaks even when called with huge Python objects. Here is a slightly contrived example:

function lookupKey(obj, key) {
   return obj.get(key);
}
pyodide.runPython(`
   from js import lookupKey
   huge_dictionary = {"some_key": 7 } # imagine a lot of other stuff in this!
   # works as expected, doesn't leak the dictionary
   val = lookupKey(huge_dictionary, "some_key")
`);

When the function does store the Python object, it causes an error but the error message can tell you useful context about the problem. Here is a nonworking example that tries to persist the dictionary:

let dict;
function setDictionary(d) {
   dict = d;
}
function lookupKey(key) {
   return dict.get(key);
}
pyodide.runPython(`
   from js import setDictionary, lookupKey
   huge_dictionary = {"some_key": 7 } # imagine a lot of other stuff in this!
   # still doesn't leak the dictionary, but it doesn't work
   setDictionary(huge_dictionary)
   lookupKey("some_key")
`);

the error says:

This borrowed proxy was automatically destroyed at the end of a function call. Try using create_proxy or create_once_callable.
The object was of type "dict" and had repr "{'some_key': 7}"

So you either have to modify the js code or the Python code. The modified js code might look like:

let dict;
function setDictionary(d) {
   if(dict) {
      dict.destroy();
   }
   if (d) {
      d = d.copy();
   }
   dict = d;
}
function lookupKey(key) {
   return dict.get(key);
}
pyodide.runPython(`
   from js import setDictionary, lookupKey
   huge_dictionary = {"some_key": 7 } # imagine a lot of other stuff in this!
   setDictionary(huge_dictionary)
   assert lookupKey("some_key") == 7
   setDictionary(None) # dictionary is freed now
`);

The modified Python code would be something like:

function setDictionary(d) {
   dict = d;
}
function lookupKey(key) {
   return dict.get(key);
}
pyodide.runPython(`
   from js import setDictionary, lookupKey
   from pyodide.ffi import create_proxy
   proxy = None
   def set_dictionary(d):
      global proxy
      if proxy:
         proxy.destroy()
      if d is not None:
         proxy = create_proxy(d)
      else:
         proxy = None
      setDictionary(proxy)

   huge_dictionary = {"some_key": 7 } # imagine a lot of other stuff in this!
   setDictionary(huge_dictionary)
   assert lookupKey("some_key") == 7
   setDictionary(None) # dictionary is freed now
`);

or of course if you just want to leak the Python objects and let FinalizationRegistry eventually do its work you can use create_proxy and forget about the proxy. I haven't figured out a good way to support both the people who are doing more memory sensitive things and the people who want everything to work as easily as possible and don't care about leaking things.

A useful exception to this is that if the function returns a promise (respectively a generator), then the arguments are kept alive until the promise is resolved (resp. the generator is exhausted). This means that an async function works as expected:

async function doSomething(pyobj) {
    pyobj.use_it();
    await something();
    // pyobj is still alive because Pyodide waits for the promise to be resolved to reclaim it
    pyobj.use_it();    
}

Of course this means that the following will leak:

function leakArsg(pyobj) {
    // Return a promise that will never resolve
    return new Promise((res, rej) => {}); 
}

from pyodide.