Skip to content
This repository was archived by the owner on Nov 12, 2022. It is now read-only.

Call JS_ShutDown when all runtimes have been dropped. #344

Merged
merged 1 commit into from
Mar 31, 2017
Merged

Call JS_ShutDown when all runtimes have been dropped. #344

merged 1 commit into from
Mar 31, 2017

Conversation

jdm
Copy link
Member

@jdm jdm commented Mar 27, 2017 •
edited by larsbergstrom
Loading

This will enable better investigation of Servo's shutdown behaviour, since currently all of the JS helper threads just sit there until process exit.

This change is Reviewable

@jdm
Copy link
Member Author

jdm commented Mar 28, 2017

r? @nox

@nox
Copy link
Contributor

nox commented Mar 28, 2017

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

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_ShutDown

@jdm
Copy link
Member Author

jdm commented Mar 28, 2017

Yes, this PR is made under the assumption that we do not create new Runtime objects after all existing Runtime objects have been dropped. I could make that explicit by panicking in Runtime::new or making it return a Result if you'd like.

@nox
Copy link
Contributor

nox commented Mar 28, 2017

Yeah, such assumptions are bad in safe code.

@jdm
Copy link
Member Author

jdm commented Mar 30, 2017

Fixed.

@nox
Copy link
Contributor

nox commented Mar 31, 2017

@bors-servo r+

@jdm
Copy link
Member Author

jdm commented Mar 31, 2017

@bors-servo: r=nox

@bors-servo
Copy link
Contributor

📌 Commit b991ef3 has been approved by nox

@bors-servo
Copy link
Contributor

⌛ Testing commit b991ef3 with merge 7f380ab...

bors-servo pushed a commit that referenced this pull request Mar 31, 2017
Auto merge of #344 - jdm:shutdown, r=nox
7f380ab 
Call JS_ShutDown when all runtimes have been dropped.

This will enable better investigation of Servo's shutdown behaviour, since currently all of the JS helper threads just sit there until process exit.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/344)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@bors-servo
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: nox
Pushing 7f380ab to master...

@bors-servo bors-servo merged commit b991ef3 into servo:master Mar 31, 2017
@chrmod
Copy link

chrmod commented Apr 9, 2017

This PR introduced a breaking change to rust-mozjs api.

Is there any publishing model you use, to help lock projects on certain version?

@tschneidereit
Copy link
Contributor

tschneidereit commented Apr 9, 2017 via email

@chrmod
Copy link

chrmod commented Apr 9, 2017

that is good to know, thanks. Is there anyway to track your progress? How far away you are from 1.0?

@jdm
Copy link
Member Author

jdm commented Apr 9, 2017

I would not bet on 1.0 being declared this year.

@tschneidereit
Copy link
Contributor

I would not bet on 1.0 being declared this year.

Agreed, it's quite unlikely it'll get stabilized this year. The API is very likely to undergo substantial changes, with stabilization beginning late this year or in 2018.

@jdm jdm mentioned this pull request Oct 15, 2018
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jdm @nox @bors-servo @chrmod @tschneidereit