weird assertion failure in js::NewContext (JS60)

[Kent Williams]

This is an app that embeds JS60 SpiderMonkey (checked out from github
and based on the the JS60 release tag)

The same code seems to work fine in a bunch of my own tests.  I passed
it off to another programmer, and he somehow gets it to segfault.

My mystery is that the stuff he's doing follows EXACTLY the same path
into the SpiderMonkey code as everything else I've done.

The crash comes on this line:

    MOZ_RELEASE_ASSERT(!TlsContext.get());

at the top of js::NewContext()

I don't know what TlsContext.get() is supposed to return, except in this
case it's returning NULL/nullptr.

Any clues.

[Paul Bone]

There's so little information here it's not going too be practical to guess.
Can you provide a stack trace? Unless you can reproduce it yourself it's
going to be very difficult to work on. The other programmer should try
doing a debug non-optimized build and running it in a debugger to try to
find the problem. Because it's your embedding they may need to work with
you in case it's a bug in your code.

Good luck.


--
Paul Bone
http://paul.bone.id.au

[tcam...@mozilla.com]

The TlsContext here is an accessor for the JSContext bound to the current thread. The assert is checking that there currently is no existing JSContext and suggests that the js::NewContext() was called twice on the same thread.

--Ted

[Kent Williams]

I found out what my problem was.

My interpreter global is this:

JS::PersistentRooted<JS Object *> global;

So since it's a JS::PersistentRootedObject, it needs this before it's
assigned a value:

global.init(context);

This will cause a problem if there is an active JSAutoRequest.  The init
has to happen before entering the request.

I found another request-related issue -- if you're building for
Debugging, Spidermonkey throws an error if you call JS::DestroyContext
with a request still outstanding.

> _______________________________________________
> dev-tech-js-engine mailing list
> dev-tech-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-js-engine

[Steve Fink]

On 3/28/19 12:34 PM, Kent Williams wrote:
> I found out what my problem was.
>
> My interpreter global is this:
>

> JS::PersistentRooted<JSObject *> global;

>
> So since it's a JS::PersistentRootedObject, it needs this before it's
> assigned a value:
>
> global.init(context);

Makes sense. I have seen other issues with the ordering of
JSContext/JSRuntime shutdown that showed up with PersistentRooted.
Unfortunately, I don't think I backported my fixes to esr60. It doesn't
look like it would have affected your case, though, since you're looking
at startup, not shutdown.

> This will cause a problem if there is an active JSAutoRequest.  The
> init has to happen before entering the request.

Requests are gone on trunk. But I'm not completely clear -- are you
still having an issue, or does it work to init before any requests?

> I found another request-related issue -- if you're building for
> Debugging, Spidermonkey throws an error if you call JS::DestroyContext
> with a request still outstanding.

Is that an issue, or a "don't do that" type of thing?

[Kent Williams]

1. The init before opening the request is fine.

2. JS::DestroyContext with active request is a dumb idea.

"Requests are gone in Trunk"

Is there any point at trying to port to ESR > 60?  And 'port' is the
proper word in my opinion. The SpiderMonkey API is a moving target.

[Steve Fink]

On 3/28/19 1:23 PM, Kent Williams wrote:
> 1. The init before opening the request is fine.
>
> 2. JS::DestroyContext with active request is a dumb idea.
>
> "Requests are gone in Trunk"
>
> Is there any point at trying to port to ESR > 60?  And 'port' is the
> proper word in my opinion. The SpiderMonkey API is a moving target.

I don't think it's different from the usual calculation. We'll stop
backporting bugfixes when the new esr comes out, and I suspect porting
across 1 esr at a time is easier than porting across 2. But those are
independent of the specific esr in question, and will matter more or
less for your particular situation.

Generally, I think we're a bit faster and less memory-intensive since
esr60, and it's easier to integrate with the GC properly. Feature-wise,
I would guess you probably wouldn't expect to make use of BigInt,
BinAST, or WASM which are all new or dramatically improved. The things
most likely to be useful to you are modules and Promises. The things
most likely to be annoying are that jsapi.h is gradually being split up
into separate headers, and the introduction of Realms.

That's my take based on a quick skim of hg history.

Hopefully porting will be a little easier with the existence of
https://github.com/spidermonkey-embedders/
<https://github.com/spidermonkey-embedders/> than it has been in the
past. (I have no quibble with calling it porting; I know how much churn
there is, and we make no pretense of interface stability anymore. I feel
like the APi churn has slowed down a fair bit, fwiw.)