Add a lifetime bound to (Safe)JSContext

[gmorenz]

JSContext::from_ptr requires that

/// - only one [JSContext] can be alive and it should not outlive [Runtime].

Currently this unsafe function is the only way to construct one, but if we're looking forward to a future where we expose a safe API we should imagine there will be a safe way to derive one from a runtime at some point... which would then require a lifetime to make sure it doesn't live to long.

It's probably a lot easier to add that now then to do the refactoring later.

(This probably should have been a review comment on #638 but too late now)

[sagudev]

That was in the initial design, but I later decided that we can do this:

mozjs/mozjs/src/rust.rs

Line 403 in 16e1e3a

pub fn cx<'rt>(&'rt mut self) -> &'rt mut crate::context::JSContext {

So instead of dealing with JSContext<'rt> we deal with &'rt JSContext. Which is needed anyway and more simpler to reason with.
Let me know if I am missing something.

[gmorenz]

Hmm, so I missed that the idea for safe construction was we just do it internally to mozjs and only hand out references.

Reaching into "why would anyone do this" territory, someone could safelystd::mem::swap(runtime1.cx(), runtime2.cx()) and violate lifetime guarantees (I assume creating two runtimes on the same thread doesn't error?). Generally not a huge fan of reference -> pointer APIs because of issues like that though I definitely get that it comes with nice things like reborrows.

[sagudev]

I assume creating two runtimes on the same thread doesn't error?

It does in

mozjs/mozjs/src/rust.rs

Line 375 in 16e1e3a

assert!(context.get().is_none());

as we store ctx in TLS.So I think that swap should not cause an problems, as you cannot really swap with anything really (at least not anything obtained safely), but it's interesting problem never the less.

The reason why I want to have ref and avoid inner lifetimes as it makes it easier (more natural) to reason about, which is especially important in servo (lifetimes are harder to understand than & vs &mut).

[gmorenz]

Alright, I think I'm satisfied that this should be ok, sorry for the noise.

Incidentally readings spidermonkey's source it looks like there's a similar check there against having two contexts at the same time on the same thread as well.

[sagudev]

Alright, I think I'm satisfied that this should be ok, sorry for the noise.

Thank you for the report anyway, it's always good to have another pair of eyes. TBH I kind of forget that swap problem exists (fortunately it's covered by another invariant but still).

Feel free to report more problems even if unsure/unfinished, as that is still better then forgetting about potential problems.