1958298 - Crash in [@ mozilla::Vector<T>::popBack | JS::GCVector<T>::popBack]

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

10 crashes from 9 installs of Firefox 137 on Windows, no crashes for Firefox 136.

5/5 crash stacks checked had been corrupted.

Jon, could you evaluate these reports?

Crash report: https://crash-stats.mozilla.org/report/index/e2c01178-4cac-49bc-81de-67f740250401

Reason:

EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  xul.dll  mozilla::Vector<JSObject*, 8, js::TempAllocPolicy>::popBack()  mfbt/Vector.h:1523
0  xul.dll  JS::GCVector<JSObject*, 8, js::TempAllocPolicy>::popBack()  js/public/GCVector.h:155
0  xul.dll  js::MutableWrappedPtrOperations<JS::GCVector<JSObject*, 8, js::TempAllocPolic...  js/public/GCVector.h:316
0  xul.dll  CycleDetector::~CycleDetector()  js/src/builtin/JSON.cpp:425
0  xul.dll  SerializeJSONObject(JSContext*, JS::Handle<JSObject*>, (anonymous namespace):...  js/src/builtin/JSON.cpp:562
0  xul.dll  SerializeJSONProperty(JSContext*, JS::Value const&, (anonymous namespace)::St...  js/src/builtin/JSON.cpp:787
1  xul.dll  SerializeJSONObject(JSContext*, JS::Handle<JSObject*>, (anonymous namespace):...  js/src/builtin/JSON.cpp:550
1  xul.dll  SerializeJSONProperty(JSContext*, JS::Value const&, (anonymous namespace)::St...  js/src/builtin/JSON.cpp:787
2  xul.dll  SerializeJSONArray(JSContext*, JS::Handle<JSObject*>, (anonymous namespace)::...  js/src/builtin/JSON.cpp:678
2  xul.dll  SerializeJSONProperty(JSContext*, JS::Value const&, (anonymous namespace)::St...  js/src/builtin/JSON.cpp:787

Comment 1

[Jon Coppeard (:jonco)]

This is not GC related.

The only change to JSON.cpp that happened in the 137 timeframe was that bug 1886882 landed which removed records and tuples. I took a look and couldn't spot anything wrong. Needinfo'ing Dan in case he can see anything.

Comment 2

[Dan Minor [:dminor]]

There's one difference I noticed that might be significant, I removed a block scope when I removed the R&T code here: https://searchfox.org/mozilla-central/diff/51417e0d53a7b270eee3488dd01f13bf360729d0/js/src/builtin/JSON.cpp#523, that would presumably change how long we keep objValue alive. There are earlier versions without this scope, and I don't think it should make a difference from reading adjacent code, but I guess it's possible that there was a latent problem that was hidden by having this block scope, that we're seeing now that's it's been removed.

I guess we could try re-introducing that scope and see if that affects this crash, I'd be happy to do that if we think it's worthwhile.

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1958298 - Reintroduce a scope in JSON.cpp; r=jonco!

Removing Records&Tuples code appears to have caused a crash. The only
change that wasn't just deleting code was the removal of a block scope
in SerializeJSONObject. The scope seems unnecessary, but if
reintroducing causes the crash to disappear, that would be a starting
point for further investigation.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c782b547d8b6
Reintroduce a scope in JSON.cpp; r=jonco

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c782b547d8b6

Comment 6

[Dan Minor [:dminor]]

Looking at the crash stats, we only saw this crash on Firefox 137. The "fix" here landed in Firefox 139, so it's unlikely that it made a difference, it's more likely something else changed in Firefox 137 and Firefox 138 that caused this crash to appear and then disappear again.