diff --git a/conftest.py b/conftest.py index dd994af84..8d24731a0 100644 --- a/conftest.py +++ b/conftest.py @@ -101,10 +101,17 @@ class SeleniumWrapper: self.run_js( """ window.pyodide = await loadPyodide({ indexURL : './', fullStdLib: false }); + pyodide.globals.get; + pyodide.pyodide_py.eval_code; + pyodide.pyodide_py.eval_code_async; + pyodide.pyodide_py.register_js_module; + pyodide.pyodide_py.unregister_js_module; + pyodide.pyodide_py.find_imports; pyodide.runPython(""); """ ) self.save_state() + self.restore_state() self.script_timeout = script_timeout self.driver.set_script_timeout(script_timeout) @@ -364,8 +371,11 @@ def pytest_runtest_call(item): if "selenium_standalone" in item._fixtureinfo.argnames: selenium = item.funcargs["selenium_standalone"] if selenium: - trace_hiwire_refs = pytest.mark.skip_refcount_check.mark not in item.own_markers trace_pyproxies = pytest.mark.skip_pyproxy_check.mark not in item.own_markers + trace_hiwire_refs = ( + trace_pyproxies + and pytest.mark.skip_refcount_check.mark not in item.own_markers + ) yield from test_wrapper_check_for_memory_leaks( selenium, trace_hiwire_refs, trace_pyproxies ) @@ -385,12 +395,13 @@ def test_wrapper_check_for_memory_leaks(selenium, trace_hiwire_refs, trace_pypro # get_result (we don't want to override the error message by raising a # different error here.) a.get_result() - if trace_pyproxies: + if trace_pyproxies and trace_hiwire_refs: delta_proxies = selenium.get_num_proxies() - init_num_proxies - assert delta_proxies == 0 + delta_keys = selenium.get_num_hiwire_keys() - init_num_keys + assert (delta_proxies, delta_keys) == (0, 0) or delta_keys < 0 if trace_hiwire_refs: delta_keys = selenium.get_num_hiwire_keys() - init_num_keys - assert delta_keys == 0 + assert delta_keys <= 0 @contextlib.contextmanager diff --git a/docs/project/changelog.md b/docs/project/changelog.md index bf5239bda..5aee8142f 100644 --- a/docs/project/changelog.md +++ b/docs/project/changelog.md @@ -86,6 +86,11 @@ substitutions: `Symbol` keys put markers on the PyProxy that can be used by external code. They will not currently be copied by `PyProxy.copy`. {pr}`1696` +- {{ Enhancement }} Memory management of `PyProxy` fields has been changed so + that fields looked up on a `PyProxy` are "borrowed" and have their lifetime + attached to the base `PyProxy`. This is intended to allow for more idiomatic + usage. + (See {issue}`1617`.) {pr}`1636` ### pyodide-build diff --git a/docs/usage/type-conversions.md b/docs/usage/type-conversions.md index e1b2c7b7d..bf28032f8 100644 --- a/docs/usage/type-conversions.md +++ b/docs/usage/type-conversions.md @@ -531,37 +531,6 @@ practice it typically takes a long time. Furthermore, the Javascript garbage collector does not have any information about whether Python is experiencing memory pressure. So it's best to aim to avoid leaks. -When using a `PyProxy`, note that accessing a field of the `PyProxy` is likely -to yield more `PyProxy` objects that also need to be destroyed. A particular -gotcha occurs with method calls: -```js -pyproxy.some_func(10); -pyproxy.destroy(); -``` -This leaks `pyproxy`! Insteaad: -```js -let some_func = pyproxy.some_func; -some_func(10); -pyproxy.destroy(); -some_func.destroy(); -``` -To be absolutely foolproof we can do it in a finally block: -```js -let some_func; -try { - some_func = pyproxy.some_func; - some_func(10); -} finlly { - // To be extra sure we do it in a finally block. - pyproxy.destroy(); - if(some_func){ - some_func.destroy(); - } -} -``` -Obviously it's not a whole lot of fun writing code like this. We hope to improve -the design to make managing `PyProxy` lifecycles more ergonomic in the future. - Here are some tips for how to do that when calling functions in one language from another. There are four cases to consider here: @@ -657,11 +626,9 @@ should destroy the argument when you are done. This can be a bit tedious to get correct due to `PyProxy` usage constraints. ```pyodide function callback(arg){ - let res_method = arg.result; - let res = res_method(); + let res = arg.result(); window.result = res.toJs(); arg.destroy(); - res_method.destroy(); res.destroy(); } let fut = pyodide.runPython(` diff --git a/src/core/README.md b/src/core/README.md index e2e17cb21..0af17ab19 100644 --- a/src/core/README.md +++ b/src/core/README.md @@ -8,12 +8,12 @@ behaviors are better defined in Python when possible for easier development and debugging. In particular, when possible logic should be moved from core/pyodide to -py/_pyodide or to py/pyodide. +py/\_pyodide or to py/pyodide. The core/pyodide code is responsible for the following main steps: 1. Initialize the CPython interpreter -2. Import py/_pyodide +2. Import py/\_pyodide 3. Initialize `_pyodide_core` which is a Python C extension that we use to make functionality available to py/pyodide. 4. Set up functionality to automatically convert functions from Javascript to diff --git a/src/core/pyproxy.c b/src/core/pyproxy.c index 3a2cf66b7..a545aa5d2 100644 --- a/src/core/pyproxy.c +++ b/src/core/pyproxy.c @@ -200,24 +200,98 @@ finally: return result; } +/* Specialized version of _PyObject_GenericGetAttrWithDict + specifically for the LOAD_METHOD opcode. + + Return 1 if a method is found, 0 if it's a regular attribute + from __dict__ or something returned by using a descriptor + protocol. + + `method` will point to the resolved attribute or NULL. In the + latter case, an error will be set. +*/ +int +_PyObject_GetMethod(PyObject* obj, PyObject* name, PyObject** method); + +// Use raw EM_JS here, we intend to raise fatal error if called on bad input. +EM_JS(int, pyproxy_Check, (JsRef x), { + if (x == 0) { + return false; + } + let val = Module.hiwire.get_value(x); + return Module.isPyProxy(val); +}); + +EM_JS(int, pyproxy_mark_borrowed, (JsRef id), { + let proxy = Module.hiwire.get_value(id); + Module.pyproxy_mark_borrowed(proxy); +}); + +EM_JS(JsRef, proxy_cache_get, (JsRef proxyCacheId, PyObject* descr), { + let proxyCache = Module.hiwire.get_value(proxyCacheId); + return proxyCache.get(descr); +}) + +// clang-format off +EM_JS(void, +proxy_cache_set, +(JsRef proxyCacheId, PyObject* descr, JsRef proxy), { + let proxyCache = Module.hiwire.get_value(proxyCacheId); + proxyCache.set(descr, proxy); +}) +// clang-format on + JsRef -_pyproxy_getattr(PyObject* pyobj, JsRef idkey) +_pyproxy_getattr(PyObject* pyobj, JsRef idkey, JsRef proxyCache) { bool success = false; PyObject* pykey = NULL; + PyObject* pydescr = NULL; PyObject* pyresult = NULL; JsRef idresult = NULL; pykey = js2python(idkey); FAIL_IF_NULL(pykey); - pyresult = PyObject_GetAttr(pyobj, pykey); - FAIL_IF_NULL(pyresult); + // If it's a method, we use the descriptor pointer as the cache key rather + // than the actual bound method. This allows us to reuse bound methods from + // the cache. + // _PyObject_GetMethod will return true and store a descriptor into pydescr if + // the attribute we are looking up is a method, otherwise it will return false + // and set pydescr to the actual attribute (in particular, I believe that it + // will resolve other types of getter descriptors automatically). + int is_method = _PyObject_GetMethod(pyobj, pykey, &pydescr); + FAIL_IF_NULL(pydescr); + JsRef cached_proxy = proxy_cache_get(proxyCache, pydescr); /* borrowed */ + if (cached_proxy) { + idresult = hiwire_incref(cached_proxy); + goto success; + } + if (PyErr_Occurred()) { + FAIL(); + } + if (is_method) { + pyresult = + Py_TYPE(pydescr)->tp_descr_get(pydescr, pyobj, (PyObject*)pyobj->ob_type); + FAIL_IF_NULL(pyresult); + } else { + pyresult = pydescr; + Py_INCREF(pydescr); + } idresult = python2js(pyresult); FAIL_IF_NULL(idresult); + if (pyproxy_Check(idresult)) { + // If a getter returns a different object every time, this could potentially + // fill up the cache with a lot of junk. However, there is no other option + // that makes sense from the point of the user. + proxy_cache_set(proxyCache, pydescr, hiwire_incref(idresult)); + pyproxy_mark_borrowed(idresult); + } +success: success = true; finally: Py_CLEAR(pykey); + Py_CLEAR(pydescr); Py_CLEAR(pyresult); if (!success) { if (PyErr_ExceptionMatches(PyExc_AttributeError)) { @@ -870,7 +944,7 @@ EM_JS_REF(JsRef, create_once_callable, (PyObject * obj), { Module.finalizationRegistry.unregister(wrapper); _Py_DecRef(obj); }; - Module.finalizationRegistry.register(wrapper, obj, wrapper); + Module.finalizationRegistry.register(wrapper, [ obj, undefined ], wrapper); return Module.hiwire.new_value(wrapper); }); diff --git a/src/core/pyproxy.h b/src/core/pyproxy.h index 2d77eedb6..aebfc8190 100644 --- a/src/core/pyproxy.h +++ b/src/core/pyproxy.h @@ -13,6 +13,9 @@ JsRef pyproxy_new(PyObject* obj); +int +pyproxy_Check(JsRef x); + /** * Wrap a Python callable in a Javascript function that can be called once. * After being called, the reference count of the python object is automatically diff --git a/src/core/pyproxy.js b/src/core/pyproxy.js index 91d6fe8cc..386ac5dbb 100644 --- a/src/core/pyproxy.js +++ b/src/core/pyproxy.js @@ -30,7 +30,8 @@ export function isPyProxy(jsobj) { Module.isPyProxy = isPyProxy; if (globalThis.FinalizationRegistry) { - Module.finalizationRegistry = new FinalizationRegistry((ptr) => { + Module.finalizationRegistry = new FinalizationRegistry(([ptr, cache]) => { + pyproxy_decref_cache(cache); try { Module._Py_DecRef(ptr); } catch (e) { @@ -76,26 +77,19 @@ Module.disable_pyproxy_allocation_tracing = function () { Module.disable_pyproxy_allocation_tracing(); /** + * Create a new PyProxy wraping ptrobj which is a PyObject*. + * + * The argument cache is only needed to implement the PyProxy.copy API, it + * allows the copy of the PyProxy to share its attribute cache with the original + * version. In all other cases, pyproxy_new should be called with one argument. + * * In the case that the Python object is callable, PyProxyClass inherits from - * Function so that PyProxy objects can be callable. - * - * The following properties on a Python object will be shadowed in the proxy - * in the case that the Python object is callable: - * - "arguments" and - * - "caller" - * - * Inheriting from Function has the unfortunate side effect that we MUST - * expose the members "proxy.arguments" and "proxy.caller" because they are - * nonconfigurable, nonwritable, nonenumerable own properties. They are just - * always `null`. - * - * We also get the properties "length" and "name" which are configurable so we - * delete them in the constructor. "prototype" is not configurable so we can't - * delete it, however it *is* writable so we set it to be undefined. We must - * still make "prototype in proxy" be true though. + * Function so that PyProxy objects can be callable. In that case we MUST expose + * certain properties inherited from Function, but we do our best to remove as + * many as possible. * @private */ -Module.pyproxy_new = function (ptrobj) { +Module.pyproxy_new = function (ptrobj, cache) { let flags = Module._pyproxy_getflags(ptrobj); let cls = Module.getPyProxyClass(flags); // Reflect.construct calls the constructor of Module.PyProxyClass but sets @@ -117,13 +111,20 @@ Module.pyproxy_new = function (ptrobj) { } else { target = Object.create(cls.prototype); } + if (!cache) { + // The cache needs to be accessed primarily from the C function + // _pyproxy_getattr so we make a hiwire id. + let cacheId = Module.hiwire.new_value(new Map()); + cache = { cacheId, refcnt: 0 }; + } + cache.refcnt++; Object.defineProperty(target, "$$", { - value: { ptr: ptrobj, type: "PyProxy" }, + value: { ptr: ptrobj, type: "PyProxy", borrowed: false, cache }, }); Module._Py_IncRef(ptrobj); let proxy = new Proxy(target, PyProxyHandlers); trace_pyproxy_alloc(proxy); - Module.finalizationRegistry.register(proxy, ptrobj, proxy); + Module.finalizationRegistry.register(proxy, [ptrobj, cache], proxy); return proxy; }; @@ -188,6 +189,46 @@ Module.getPyProxyClass = function (flags) { // Static methods Module.PyProxy_getPtr = _getPtr; +Module.pyproxy_mark_borrowed = function (proxy) { + proxy.$$.borrowed = true; +}; + +const pyproxy_cache_destroyed_msg = + "This borrowed attribute proxy was automatically destroyed in the " + + "process of destroying the proxy it was borrowed from. Try using the 'copy' method."; + +function pyproxy_decref_cache(cache) { + if (!cache) { + return; + } + cache.refcnt--; + if (cache.refcnt === 0) { + let cache_map = Module.hiwire.pop_value(cache.cacheId); + for (let proxy_id of cache_map.values()) { + Module.pyproxy_destroy( + Module.hiwire.pop_value(proxy_id), + pyproxy_cache_destroyed_msg + ); + } + } +} + +Module.pyproxy_destroy = function (proxy, destroyed_msg) { + let ptrobj = _getPtr(proxy); + Module.finalizationRegistry.unregister(proxy); + // Maybe the destructor will call Javascript code that will somehow try + // to use this proxy. Mark it deleted before decrementing reference count + // just in case! + proxy.$$.ptr = null; + proxy.$$.destroyed_msg = destroyed_msg; + pyproxy_decref_cache(proxy.$$.cache); + try { + Module._Py_DecRef(ptrobj); + trace_pyproxy_dealloc(proxy); + } catch (e) { + Module.fatal_error(e); + } +}; // Now a lot of boilerplate to wrap the abstract Object protocol wrappers // defined in pyproxy.c in Javascript functions. @@ -285,25 +326,15 @@ class PyProxyClass { * `_ * Pyodide will automatically destroy the ``PyProxy`` when it is garbage * collected, however there is no guarantee that the finalizer will be run - * in a timely manner so it is better to ``destory`` the proxy explicitly. + * in a timely manner so it is better to ``destroy`` the proxy explicitly. * * @param {string} [destroyed_msg] The error message to print if use is * attempted after destroying. Defaults to "Object has already been * destroyed". */ destroy(destroyed_msg) { - let ptrobj = _getPtr(this); - Module.finalizationRegistry.unregister(this); - // Maybe the destructor will call Javascript code that will somehow try - // to use this proxy. Mark it deleted before decrementing reference count - // just in case! - this.$$.ptr = null; - this.$$.destroyed_msg = destroyed_msg; - try { - Module._Py_DecRef(ptrobj); - trace_pyproxy_dealloc(this); - } catch (e) { - Module.fatal_error(e); + if (!this.$$.borrowed) { + Module.pyproxy_destroy(this, destroyed_msg); } } /** @@ -313,7 +344,7 @@ class PyProxyClass { */ copy() { let ptrobj = _getPtr(this); - return Module.pyproxy_new(ptrobj); + return Module.pyproxy_new(ptrobj, this.$$.cache); } /** * Converts the ``PyProxy`` into a Javascript object as best as possible. By @@ -647,7 +678,7 @@ class PyProxyIterableMethods { } let result = iter_helper(iterptr, token); - Module.finalizationRegistry.register(result, iterptr, token); + Module.finalizationRegistry.register(result, [iterptr, undefined], token); return result; } } @@ -734,8 +765,9 @@ function python_getattr(jsobj, jskey) { let ptrobj = _getPtr(jsobj); let idkey = Module.hiwire.new_value(jskey); let idresult; + let cacheId = jsobj.$$.cache.cacheId; try { - idresult = Module.__pyproxy_getattr(ptrobj, idkey); + idresult = Module.__pyproxy_getattr(ptrobj, idkey, cacheId); } catch (e) { Module.fatal_error(e); } finally { diff --git a/src/js/api.js b/src/js/api.js index 40f3c9cc5..adf909fc0 100644 --- a/src/js/api.js +++ b/src/js/api.js @@ -90,12 +90,7 @@ export let version = ""; // actually defined in runPythonSimple in loadPyodide ( * documentation for :any:`pyodide.eval_code` for more info. */ export function runPython(code, globals = Module.globals) { - let eval_code = Module.pyodide_py.eval_code; - try { - return eval_code(code, globals); - } finally { - eval_code.destroy(); - } + return Module.pyodide_py.eval_code(code, globals); } Module.runPython = runPython; @@ -132,15 +127,12 @@ export async function loadPackagesFromImports( messageCallback, errorCallback ) { - let find_imports = Module.pyodide_py.find_imports; + let pyimports = Module.pyodide_py.find_imports(code); let imports; - let pyimports; try { - pyimports = find_imports(code); imports = pyimports.toJs(); } finally { - find_imports.destroy(); - pyimports && pyimports.destroy(); + pyimports.destroy(); } if (imports.length === 0) { return; @@ -199,13 +191,11 @@ export function pyimport(name) { * @async */ export async function runPythonAsync(code) { - let eval_code_async = Module.pyodide_py.eval_code_async; - let coroutine = eval_code_async(code, Module.globals); + let coroutine = Module.pyodide_py.eval_code_async(code, Module.globals); try { let result = await coroutine; return result; } finally { - eval_code_async.destroy(); coroutine.destroy(); } } @@ -223,12 +213,7 @@ Module.runPythonAsync = runPythonAsync; * @param {object} module Javascript object backing the module */ export function registerJsModule(name, module) { - let register_js_module = Module.pyodide_py.register_js_module; - try { - register_js_module(name, module); - } finally { - register_js_module.destroy(); - } + Module.pyodide_py.register_js_module(name, module); } /** @@ -251,12 +236,7 @@ export function registerComlink(Comlink) { * @param {string} name Name of the Javascript module to remove */ export function unregisterJsModule(name) { - let unregister_js_module = Module.pyodide_py.unregister_js_module; - try { - unregister_js_module(name); - } finally { - unregister_js_module.destroy(); - } + Module.pyodide_py.unregister_js_module(name); } /** diff --git a/src/tests/test_asyncio.py b/src/tests/test_asyncio.py index 35dad36d0..61434d93b 100644 --- a/src/tests/test_asyncio.py +++ b/src/tests/test_asyncio.py @@ -353,11 +353,9 @@ def test_await_pyproxy_eval_async(selenium): assert ( selenium.run_js( """ - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async("1+1"); + let c = pyodide.pyodide_py.eval_code_async("1+1"); let result = await c; c.destroy(); - eval_code_async.destroy(); return result; """ ) @@ -368,11 +366,9 @@ def test_await_pyproxy_eval_async(selenium): selenium.run_js( """ let finally_occurred = false; - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async("1+1"); + let c = pyodide.pyodide_py.eval_code_async("1+1"); let result = await c.finally(() => { finally_occurred = true; }); c.destroy(); - eval_code_async.destroy(); return [result, finally_occurred]; """ ) @@ -384,14 +380,12 @@ def test_await_pyproxy_eval_async(selenium): """ let finally_occurred = false; let err_occurred = false; - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async("raise ValueError('hi')"); + let c = pyodide.pyodide_py.eval_code_async("raise ValueError('hi')"); try { let result = await c.finally(() => { finally_occurred = true; }); } catch(e){ err_occurred = e.constructor.name === "PythonError"; } - eval_code_async.destroy(); c.destroy(); return [finally_occurred, err_occurred]; """ @@ -401,9 +395,7 @@ def test_await_pyproxy_eval_async(selenium): assert selenium.run_js( """ - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async("raise ValueError('hi')"); - eval_code_async.destroy(); + let c = pyodide.pyodide_py.eval_code_async("raise ValueError('hi')"); try { return await c.catch(e => e.constructor.name === "PythonError"); } finally { @@ -414,13 +406,11 @@ def test_await_pyproxy_eval_async(selenium): assert selenium.run_js( """ - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async(` + let c = pyodide.pyodide_py.eval_code_async(` from js import fetch await (await fetch('packages.json')).json() `); let packages = await c; - eval_code_async.destroy(); c.destroy(); return (!!packages.dependencies) && (!!packages.import_name_to_package_name); """ @@ -428,10 +418,8 @@ def test_await_pyproxy_eval_async(selenium): assert selenium.run_js( """ - let eval_code_async = pyodide.pyodide_py.eval_code_async; - let c = eval_code_async("1+1"); + let c = pyodide.pyodide_py.eval_code_async("1+1"); await c; - eval_code_async.destroy(); c.destroy(); let err_occurred = false; try { diff --git a/src/tests/test_jsproxy.py b/src/tests/test_jsproxy.py index 7c679c2fc..7f95fdb16 100644 --- a/src/tests/test_jsproxy.py +++ b/src/tests/test_jsproxy.py @@ -347,6 +347,7 @@ def test_window_isnt_super_weird_anymore(): @pytest.mark.skip_refcount_check +@pytest.mark.skip_pyproxy_check def test_mount_object(selenium_standalone): selenium = selenium_standalone result = selenium.run_js( @@ -430,6 +431,7 @@ def test_unregister_jsmodule_error(selenium): @pytest.mark.skip_refcount_check +@pytest.mark.skip_pyproxy_check def test_nested_import(selenium_standalone): selenium = selenium_standalone assert ( @@ -451,6 +453,7 @@ def test_nested_import(selenium_standalone): @pytest.mark.skip_refcount_check +@pytest.mark.skip_pyproxy_check def test_register_jsmodule_docs_example(selenium_standalone): selenium = selenium_standalone selenium.run_js( diff --git a/src/tests/test_pyodide.py b/src/tests/test_pyodide.py index 3efc60191..d6f21015c 100644 --- a/src/tests/test_pyodide.py +++ b/src/tests/test_pyodide.py @@ -148,6 +148,7 @@ def test_eval_code_locals(): eval_code("invalidate_caches()", globals, locals) +@pytest.mark.skip_pyproxy_check def test_monkeypatch_eval_code(selenium): try: selenium.run( diff --git a/src/tests/test_pyproxy.py b/src/tests/test_pyproxy.py index a3ce0403c..16eca1845 100644 --- a/src/tests/test_pyproxy.py +++ b/src/tests/test_pyproxy.py @@ -142,18 +142,15 @@ def test_pyproxy_destroy(selenium): f = Foo() """ ) - msg = "Object has already been destroyed" - with pytest.raises(selenium.JavascriptException, match=msg): - selenium.run_js( - """ - let f = pyodide.globals.get('f'); - let f_get_value = f.get_value; - assert(()=> f_get_value(1) === 64); - f_get_value.destroy(); - f.destroy(); - f.get_value(); - """ - ) + + selenium.run_js( + """ + let f = pyodide.globals.get('f'); + assert(()=> f.get_value(1) === 64); + f.destroy(); + assertThrows(() => f.get_value(1), "Error", "already been destroyed"); + """ + ) def test_pyproxy_iter(selenium): @@ -398,23 +395,21 @@ def test_pyproxy_mixins(selenium): def test_pyproxy_mixins2(selenium): selenium.run_js( """ - assert(() => !("prototype" in pyodide.globals)); - assert(() => !("caller" in pyodide.globals)); - assert(() => !("name" in pyodide.globals)); - assert(() => "length" in pyodide.globals); - let get_method = pyodide.globals.__getitem__; - assert(() => "prototype" in get_method); - assert(() => get_method.prototype === undefined); - assert(() => !("length" in get_method)); - assert(() => !("name" in get_method)); - get_method.destroy(); - let d = pyodide.runPython("{}"); - let d_get = d.$get; - assert(() => d_get.type === "builtin_function_or_method"); + + assert(() => !("prototype" in d)); + assert(() => !("caller" in d)); + assert(() => !("name" in d)); + assert(() => "length" in d); + + assert(() => "prototype" in d.__getitem__); + assert(() => d.__getitem__.prototype === undefined); + assert(() => !("length" in d.__getitem__)); + assert(() => !("name" in d.__getitem__)); + + assert(() => d.$get.type === "builtin_function_or_method"); assert(() => d.get.type === undefined); assert(() => d.set.type === undefined); - d_get.destroy(); d.destroy(); """ ) @@ -628,16 +623,14 @@ def test_pyproxy_gc_destroy(selenium): get_ref_count(2); d.get(); get_ref_count(3); - d.destroy(); - get_ref_count(4); - gc(); - get_ref_count(5); + delete d; + get_ref_count.destroy(); """ ) selenium.driver.execute_cdp_cmd("HeapProfiler.collectGarbage", {}) selenium.run( """ - get_ref_count(6) + get_ref_count(4) del d """ ) @@ -646,10 +639,8 @@ def test_pyproxy_gc_destroy(selenium): 0: 2, 1: 3, 2: 4, - 3: 5, - 4: 4, - 5: 4, - 6: 2, + 3: 4, + 4: 2, "destructor_ran": True, } @@ -826,29 +817,29 @@ def test_pyproxy_call(selenium): msg = r"TypeError: f\(\) got multiple values for argument 'x'" with pytest.raises(selenium.JavascriptException, match=msg): selenium.run_js("f.callKwargs(76, {x : 6})") + selenium.run_js("f.destroy()") -def test_pyproxy_name_clash(selenium): +def test_pyproxy_borrow(selenium): selenium.run_js( """ - let d = pyodide.runPython("{'a' : 2}"); - let d_get = d.$get; - assert(() => d.get('a') === 2); - assert(() => d_get('b', 3) === 3); - d_get.destroy(); - d.destroy(); - let t = pyodide.runPython(` - class Test: - def destroy(self): + class Tinner: + def f(self): return 7 - Test() + class Touter: + T = Tinner() + Touter `); - let t_dest = t.$destroy; - assert(() => t_dest() === 7); - t_dest.destroy(); + assert(() => t.T.f() === 7); + let T = t.T; + let Tcopy = T.copy(); + assert(() => T.f() === 7); + assert(() => Tcopy.f() === 7); t.destroy(); - assertThrows(() => t.$destroy, "Error", "Object has already been destroyed"); + assert(() => Tcopy.f() === 7); + assertThrows(() => T.f(), "Error", "automatically destroyed in the process of destroying the proxy it was borrowed from"); + Tcopy.destroy(); """ ) diff --git a/src/tests/test_typeconversions.py b/src/tests/test_typeconversions.py index 41681fe30..982a9cfb9 100644 --- a/src/tests/test_typeconversions.py +++ b/src/tests/test_typeconversions.py @@ -207,7 +207,7 @@ def test_hyp_tojs_no_crash(selenium_module_scope, obj): ) -def test_python2js(selenium): +def test_python2js1(selenium): assert selenium.run_js('return pyodide.runPython("None") === undefined') assert selenium.run_js('return pyodide.runPython("True") === true') assert selenium.run_js('return pyodide.runPython("False") === false') @@ -219,6 +219,9 @@ def test_python2js(selenium): assert selenium.run_js('return pyodide.runPython("\'ιωδιούχο\'") === "ιωδιούχο"') assert selenium.run_js('return pyodide.runPython("\'碘化物\'") === "碘化物"') assert selenium.run_js('return pyodide.runPython("\'🐍\'") === "🐍"') + + +def test_python2js2(selenium): assert selenium.run_js( """ let xpy = pyodide.runPython("b'bytes'"); @@ -229,6 +232,9 @@ def test_python2js(selenium): (x[0] === 98) """ ) + + +def test_python2js3(selenium): assert selenium.run_js( """ let proxy = pyodide.runPython("[1, 2, 3]"); @@ -239,6 +245,9 @@ def test_python2js(selenium): (x.length === 3) && (x[0] == 1) && (x[1] == 2) && (x[2] == 3)); """ ) + + +def test_python2js4(selenium): assert selenium.run_js( """ let proxy = pyodide.runPython("{42: 64}"); @@ -248,13 +257,14 @@ def test_python2js(selenium): return (typename === "dict") && (x.constructor.name === "Map") && (x.get(42) === 64); """ ) + + +def test_python2js5(selenium): assert selenium.run_js( """ let x = pyodide.runPython("open('/foo.txt', 'wb')") - let x_tell = x.tell; - let result = x_tell(); + let result = x.tell(); x.destroy(); - x_tell.destroy(); return result === 0; """ ) @@ -374,7 +384,6 @@ def test_js2python(selenium): jsfloats : new Float32Array([1, 2, 3]), jsobject : new XMLHttpRequest(), }; - Object.assign(window, test_objects); """ ) selenium.run("from js import test_objects as t")