servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.
Mozilla Public License 2.0
293 stars 122 forks source link

Store the JSContext in TLS for easy access. #322

Closed Ms2ger closed 7 years ago

Ms2ger commented 7 years ago

This change is Reviewable

nox commented 7 years ago

Why not put the TLS thing in servo/servo? Why forbid the creation of two unrelated runtimes on a single thread?

fitzgen commented 7 years ago

Why forbid the creation of two unrelated runtimes on a single thread?

SpiderMonkey requires a 1:1 relationship between a JSRuntime and its thread.

nox commented 7 years ago

@fitzgen Thanks!

@Ms2ger Could you make ::get() panic if the pointer is null?

nox commented 7 years ago

@bors-servo r+

bors-servo commented 7 years ago

:pushpin: Commit c8ebc6d has been approved by nox

bors-servo commented 7 years ago

:hourglass: Testing commit c8ebc6d with merge 1941f1c...

bors-servo commented 7 years ago

:sunny: Test successful - status-appveyor, status-travis

nox commented 7 years ago

I still believe that was the wrong way to go, because now if we want to use the Runtime value and Runtime::get to do safe abstractions around for example ToJsValConvertible, we still won't be able to avoid some unsafe code in servo for dereferencing the result of Runtime::get, whereas we could make it be owned by TLS if it were done in servo instead of here.