Skip to content

Conversation

@sagudev
Copy link
Member

@sagudev sagudev commented Oct 31, 2025

Somehow related to #554, but it creates more strict JSContext. Companion PR in servo: servo/servo#40465

More concrete documentation is written at types: https://github.com/sagudev/mozjs/blob/cx/mozjs/src/context.rs and also includes some compile tests. Here I will only describe rough idea and motivation.

If we want to build any proper safe wrappers we need to have a notion of GC/NoGC, JSContext somehow fulfills the role of GC (generally every SM function that can trigger GC takes JSContext). But to be able to use JSContext (and it's lifetime) for this notion we need to make it not globally obtainable obtainable (it needs to be passed down as an arg as mentioned in https://github.com/asajeffrey/josephine). This is already happening with CanGC we have in servo (this new JSContext would replace CanGC).

This PR also introduce new wrappers that use lifetimed handles and new safe JSContext. More specifically they use &mut JSContext (for functions that can trigger GC) or &JSContext (for functions that cannot trigger GC). We also remove https://doc.servo.org/mozjs/jsapi/struct.AutoRequireNoGC.html from args (as NoGC is already encoded in &JSContext argument).

While this model is safe and usable from mozjs, it's somehow less usable in servo, where Runtime is always part of some *GlobalScope dom_struct or TLS. So in practice all entrypoints (where we get initial cx for passdown) will likely need to be unbounded (unsafe).

The plan forward

The idea is that we can incrementally introduce safety to servo:

  • use known list of GC/NoGC functions Add safe JSContext wrapper with strict safety rules #638 (comment)
  • start using and passing down this new JSContext everywhere in servo (this will also replace can_gc effort) and use wrappers2
    • it make make more sense to pass down &mut InRealm (which is JSAutoRealm that has cx inside)
  • write some more wrappers mainly we need one for JSAutoRealm, js_str stuff, to_jsval, RootingGuard, ...
  • Handles/Roots would use NoGC to create & of inner data
  • DomRefCell would also take NoGC on borrow, borrow_mut to prevent borrows across GC.
  • I think we could also write part of unrooted with this, when creating unrooted value it would have 'cx lifetime (bounded to cx) and rooting would erase the lifetime (make it 'static thus unbound it from cx).
    • Something like this is already presented in https://github.com/asajeffrey/josephine
    • The biggest problems are return values, which also need to be bounded. This will require some additional work in mozjs

Notes on the model

While GC is run only on some compartments/realm, this becomes complicated to model because of cross references. So this model assumes all compartments in cx will get traced/GC (even if that is not the case; but that does happen on full GC I think). This is more strict than per compartments, but much easier to model and reason about (look at how complicated https://github.com/asajeffrey/josephine is). We can always introduced more relaxed model later.

This model is already discussed in #520, #560, #569 and something similar is also used in nova js engine: https://trynova.dev/blog/guide-to-nova-gc (I borrowed idea of explicit NoGC token).

@sagudev sagudev changed the title Add safe JSContext wrapper with stritct safety rules Add safe JSContext wrapper with strict safety rules Oct 31, 2025
@sagudev

This comment was marked as outdated.

@sagudev sagudev mentioned this pull request Nov 3, 2025
@sagudev sagudev force-pushed the cx branch 2 times, most recently from 3fca351 to 1a477d7 Compare November 5, 2025 18:30
@sagudev
Copy link
Member Author

sagudev commented Nov 6, 2025

Yesterday I learned that searchfox actually has trigger gc/not gc labels on functions: https://matrix.to/#/!CZEbuMfGYVdQMIBKeP:mozilla.org/$gMT8sxO85ePN_QOJ6Paa0q-nRILwXATU2iJ_EzoNhq8

These are generated by CI, so I wonder if we could use this data for autogenerating wrappers.

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev sagudev marked this pull request as ready for review November 6, 2025 16:29
@sagudev sagudev requested a review from jdm November 6, 2025 16:30
@sagudev
Copy link
Member Author

sagudev commented Nov 6, 2025

cc @gterzian @Taym95 who might also be interested in this

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exciting and very promising! My only complaint is the new & syntax in the rooting macros, but I don't actually have any suggestions.

@sagudev
Copy link
Member Author

sagudev commented Nov 7, 2025

My only complaint is the new & syntax in the rooting macros, but I don't actually have any suggestions.

Eventually when we will use safe JSContext in more places we will, we will make RootingGuard to take &JSContext (IIRC it does not GC), so all & will become redundant, but we are not there yet, so I've tried to find something minimal to minimize the diff.

@sagudev sagudev added this pull request to the merge queue Nov 7, 2025
Merged via the queue into servo:main with commit e6f40eb Nov 7, 2025
36 checks passed
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Nov 7, 2025
This PR is companion to servo/mozjs#638,
currently it only make stuff compile. In follwups we will start
passing/using safer JSContext down everywhere.

Testing: Not needed as it's just refactorings.

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
github-merge-queue bot pushed a commit to servo/servo that referenced this pull request Nov 9, 2025
This PR is companion to servo/mozjs#638,
currently it only make stuff compile. In follwups we will start
passing/using safer JSContext down everywhere.

Testing: Not needed as it's just refactorings.
try run:
https://github.com/sagudev/servo/actions/runs/19196312972/job/54879090390

---------

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev sagudev deleted the cx branch November 11, 2025 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants