Prevent initializing and shutting down SpiderMonkey from different threads

[jdm]

Based on this comment, it appears that JS_Init and JS_ShutDown need to be called on the same thread. These changes make JSEngine a non-sendable type, and add JSEngineHandle as a sendable type that allows verifying that all outstanding consumers of the engine are dropped before shutting down.

[jdm]

Note to self: need to update unit tests.

[gterzian]

To elaborate on https://github.com/servo/servo/pull/24845#issuecomment-557863826

I think you could make outstanding_handles: Arc<AtomicU32> into a outstanding_handles: Arc<(Mutex<EngineHandles>, Condvar)>.

with EngineHandles being:

enum EngineHandles {
    /// Initial state, equivalent to 0 handles, but before any handles have been created.
    Initialized,
    /// Count handles, start at 1, goes to 0 when all handles are dropped.
    Count(u32)
}

with can_shutdown containing the equivalent of while outstanding_handles != EngineHandles::Count(0) {//wait on the condvar}.

So that the thread in script can then block on that call, and then shutdown.

[jdm]

The idea of making shutdown be a blocking operation is interesting, but I think I'd rather not make that a requirement of using this crate. It's not difficult to imagine a consumer writing code that uses this API that ends up creating a deadlock by not ensuring that all of the engine handles are disposed of before attempting to shutdown. I would rather provide an API that panics by default and allow consumers to build their own blocking shutdown behaviour on top of it, like https://github.com/servo/servo/pull/24862 does for Servo.

[gterzian]

I would rather provide an API that panics by default and allow consumers to build their own blocking shutdown behaviour on top of it

Yes that makes sense, although it's not clear to me how a non-blocking use, or a blocking use that isn't a spinlock waiting for it to return true, would look like.

It would be possible for a consumer to deadlock on a blocking shutdown operation, and such deadlock is likely to be not intermittent and quickly fixed. On the other hand, a non-blocking can_shutdown could lead to intermittent issues.

I guess either way for the API could be a valid choice thought...

[jdm]

@bors-servo r=asajeffrey

[bors-servo]

:pushpin: Commit c8c1453 has been approved by asajeffrey

[bors-servo]

:hourglass: Testing commit c8c14531792ceb2d12c64a9a77dbfe087dcf8ae2 with merge 9b0d063ba062f4cc60c3bab9250218d6935d647b...

[bors-servo]

:sunny: Test successful - checks-travis, status-appveyor Approved by: asajeffrey Pushing 9b0d063ba062f4cc60c3bab9250218d6935d647b to master...