Introduce feature to only allow initialization once.

servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.

Mozilla Public License 2.0

293 stars 117 forks source link

Introduce feature to only allow initialization once. #447

Closed paulrouget closed 6 years ago

paulrouget commented 6 years ago

See #22039

This change is

jdm commented 6 years ago

@bors-servo r+

bors-servo commented 6 years ago

:pushpin: Commit 2ccdca9 has been approved by jdm

bors-servo commented 6 years ago

:hourglass: Testing commit 2ccdca91bc5346c04e224c29cf841ef1899c6348 with merge 5baf536209468213c759ccf5fe8229bd1f5f3469...

CYBAI commented 6 years ago

servo/servo#22039

(Just help to reference the issue correctly)

bors-servo commented 6 years ago

:sunny: Test successful - status-appveyor, status-travis Approved by: jdm Pushing 5baf536209468213c759ccf5fe8229bd1f5f3469 to master...

paulrouget commented 6 years ago

@CYBAI I can't see the commits from PR #445

CYBAI commented 6 years ago

Hmm, @paulrouget can you see https://github.com/servo/rust-mozjs/pull/445/commits/9d5d8a9438e06fc025d927e12a1baf4b71647352 in the PR?

Btw, mozjs on crates.io is already on 0.9.4 (https://crates.io/crates/mozjs). Not sure why GitHub didn't show the commit 🤔

paulrouget commented 6 years ago

Can't see the commit here: https://github.com/servo/rust-mozjs/commits/master/Cargo.toml but I see it here: https://github.com/servo/rust-mozjs/commits/master

Anyway. Gonna bump to 0.9.5 and publish.

nox commented 5 years ago

Why not use std::sync::Once, and why is this feature only needed on Android in Servo? AFAIK SM should never be initialised multiple times, on all platforms, so I don't understand why this behaviour is enabled by a feature.

nox commented 5 years ago

Cc @asajeffrey, who reverted most of https://github.com/servo/rust-mozjs/commit/066b4abb951841dd97b2c3a312818800fcb72131 when he updated to SM 60 in #430.

nox commented 5 years ago

From SpiderMonkey's own documentation:

It is currently not possible to initialize SpiderMonkey multiple times (that is, calling JS_Init, JSAPI methods, then JS_ShutDown in that order, then doing so again). This restriction may eventually be lifted.

JS_Init should never be called multiple times, so this shouldn't be a feature at all.

jdm commented 5 years ago

It is specific to the way that we're using Servo inside Firefox Reality. The feature simply prevents us from calling shut down; it doesn't actually change how many times we initialize the library.

asajeffrey commented 5 years ago

Hmm, might be worth finding another name for the feature? It seems to be more about shutdown than init, perhaps never_shutdown would be better?

jdm commented 5 years ago

I have an open pull request that removes it entirely.