1679094 - Resolving `dom::Promise` from rust would be cool.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[u480271]

In the work to upgrade fluent usage in Gecko, it would be useful to be able resolve dom::Promise from rust code.

Comment 1

[u480271]

I spoke with :Nika over chat about this and we came up with a plan to add very limited support using nsIVariant to handle the marshaling of data to JS.

Comment 2

[u480271]

Attachment: Bug 1679094 - Resolve dom::Promise from Rust. r?nika

A very limited interface to allow resolving dom::Promises from Rust code by
marshaling values via nsIVariant.

Depends on D97904

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cccdff68c790
Resolve dom::Promise from Rust. r=nika

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for causing build bustage

backout: https://hg.mozilla.org/integration/autoland/rev/6d4142b13ad21e136eac8b99993ab0b09ffe109f

push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=cccdff68c790f1a1e90396d5e04d14fe66c1ac90

failure log: https://treeherder.mozilla.org/logviewer?job_id=324314108&repo=autoland&lineNumber=1714

[task 2020-12-12T01:12:31.210Z] 01:12:31 INFO - make[3]: Entering directory '/builds/worker/workspace/obj-build'
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - ./cbindgen-metadata.json.stub
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - /builds/worker/workspace/obj-build/_virtualenvs/init_py3/bin/python -m mozbuild.action.file_generate /builds/worker/checkouts/gecko/build/RunCbindgen.py generate_metadata config/cbindgen-metadata.json config/.deps/cbindgen-metadata.json.pp config/.deps/cbindgen-metadata.json.stub .cargo/config
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - error: failed to select a version for the requirement cstr = "^0.1"
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - candidate versions found which didn't match: 0.2.8
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - location searched: directory source /builds/worker/checkouts/gecko/third_party/rust (which is replacing registry https://github.com/rust-lang/crates.io-index)
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - required by package xpcom v0.1.0 (/builds/worker/checkouts/gecko/xpcom/rust/xpcom)
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - perhaps a crate was updated and forgotten to be re-vendored?
[task 2020-12-12T01:12:31.211Z] 01:12:31 ERROR - make[3]: *** [backend.mk:115: config/.deps/cbindgen-metadata.json.stub] Error 101
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - make[3]: Leaving directory '/builds/worker/workspace/obj-build'
[task 2020-12-12T01:12:31.211Z] 01:12:31 INFO - make[3]: *** Waiting for unfinished jobs....

Comment 6

[u480271]

Ok, I'll update the crate dependency.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c65a4d976a90
Resolve dom::Promise from Rust. r=nika

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/c65a4d976a90

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

What is the expected memory management model here? Promise is a cycle collectable object so whoever owns it for non-trivial time should inform about the edge to the cycle collector?

Comment 10

[Nika Layzell [:nika] (ni? for response)]

(In reply to Olli Pettay [:smaug] from comment #9)

What is the expected memory management model here? Promise is a cycle collectable object so whoever owns it for non-trivial time should inform about the edge to the cycle collector?

Rust objects have no support for the cycle collector right now, so there's no expected support for supporting unlinking promise cycles here. This is similar to the situation Rust is in with other JS xpcom interface objects, where the JS object should likely be cycle-collected. This isn't a great situation, and it definitely means it's possible to write leaky objects in Rust code, but it already was possible to do this with bare xpcom objects. The risk here is somewhat limited due to Rust not being allowed to implement WebIDL-exposed interfaces, and instead being limited to xpcom objects, meaning we are less likely to run into strange JS callers which trip us up. The risk is still definitely there though.

I don't think adding dom::Promise bindings makes the situation here meaningfully worse than it was before, and it makes the ergonomics for the common situation where dom::Promise bindings are requested much nicer. The majority of rust APIs which want to expose a dom::Promise are ones which dispatch some work to be performed on a background thread, and then fulfill the promise once that work is completed. In these cases we have no choice but to keep the dom::Promise object rooted until the reply comes back, and due to the multithreaded nature, we would be unable to cycle-collect through it even in C++ code.

I think there's a lot of work to be done to make our Rust binding APIs better and less error-prone to use, and perhaps some documentation could help make it clear that holding these objects can cause memory leaks.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

I wonder if some particular kind of naming could hint to the code authors and reviewers that they are dealing with objects which can keep
worlds alive.

And one problem with these rust->C++ edges is that they won't even show up in the CC log, so debugging the leaks will be harder.