Make runtime creation safe

servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.

Mozilla Public License 2.0

293 stars 122 forks source link

Make runtime creation safe #450

Closed jdm closed 5 years ago

jdm commented 5 years ago

The fundamental problem exposed in https://github.com/servo/servo/issues/22342 is that our concept of a parent runtime did not match reality. Using the first JSContext's runtime as the global parent for all subsequent contexts only makes sense if that JSContext outlives every other context. This is not guaranteed, leading to crashes when trying to use those contexts if the first context (and therefore its runtime) was destroyed.

The new design incorporates several changes for safer, more clear context and runtime management:

in order to create a new context, either a handle to an initialized JS engine is required or a handle to an existing runtime
all runtimes track outstanding handles that have been created, and assert if a runtime is destroyed before all of its child runtimes
overall initialization and shutdown of the engine is controlled by the lifetime of a JSEngine value, so creating a Runtime value is now infallible

This change is

jdm commented 5 years ago

There is one hole that I've come up with so far - while the assertion in the Runtime drop implementation catches the point at which a parent runtime is dropped before all of its children are dropped, that only interrupts the thread on which the parent runtime is executing. The threads on which any child runtimes are executing have no signal that their parent runtime is now invalid. The best choice at this point would be to abort the whole program for the sake of safety, but this PR does not do that right now.

jdm commented 5 years ago

I fixed the racing issues by switching to a single mutex around an enum that represents the engine state. This is ready for review.

jdm commented 5 years ago

r? @asajeffrey

jdm commented 5 years ago

@asajeffrey Review comments addressed. I've added more comments around the adjusted refcounting bits to hopefully make clear why it looks weird.

asajeffrey commented 5 years ago

LGTM. Squash and r=me?

jdm commented 5 years ago

@bors-servo r=asajeffrey