servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.
Mozilla Public License 2.0
293 stars 122 forks source link

Make high-level wrappers that require &JSAutoCompartment arguments for APIs that need an active compartment #316

Open jdm opened 7 years ago

jdm commented 7 years ago

Now that promises are becoming more popular in Servo, several people without JSAPI experience are experiencing crashes because they're manipulating promises from async events that don't have an active JS compartment. It would be a huge improvement if this were impossible.

KiChjang commented 7 years ago

Huh, taking in an &JSAutoCompartment means that the caller is responsible of exiting the compartment, meaning the developer still needs to be aware of properly scoping the JSAutoCompartment variable, releasing it when it is not needed. I'm thinking of making them accept a JSAutoCompartment instead, but that also seems like a bad idea, since that's an unintended side effect.

jdm commented 7 years ago

I don't see the issue here; this API design would encode the existing semantics of using compartments (ie. a caller could have already entered a compartment; my code needs to be deliberate about entering a compartment if there's a particular compartment that needs to be entered), while being more explicit about passing-the-buck to the caller versus ensuring the correct compartment is entered when necessary.

KiChjang commented 7 years ago

I'm just wondering whether it'd be also great if we can fully utilize Rust's ownership features to also hide the details of "entering a compartment" because it's not obvious what it does to a newcomer, unless you already had experiences with the JS engine, although maybe I'm being a little bit too idealistic here.

jdm commented 7 years ago

I'm open to ideas, but transferring ownership of a JSAutoCompartment breaks the RAII style of use that it's traditionally for.