1631970 - Intermittent hazards | unrooted 'response' of type 'JS::Value' live across GC call at dom/indexedDB/ActorsChild.cpp:2711

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: btara [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=298685840&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/N9BjTnyPSIa7618DPLv_4Q/runs/0/artifacts/public/logs/live_backing.log

---

...
[task 2020-04-21T21:29:53.990Z] Running explain to generate ('hazards.txt', 'unnecessary.txt', 'refs.txt')
[task 2020-04-21T21:29:53.990Z] PATH="/builds/worker/fetches/sixgill/usr/bin:${PATH}" LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed' SOURCE='/builds/worker/checkouts/gecko' XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' python2.7 /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/explain.py rootingHazards.txt gcFunctions.txt hazards.txt unnecessary.txt refs.txt
[task 2020-04-21T21:29:56.613Z] Wrote explained_hazards.tmp
[task 2020-04-21T21:29:56.613Z] Wrote unnecessary.tmp
[task 2020-04-21T21:29:56.613Z] Wrote refs.tmp
[task 2020-04-21T21:29:56.613Z] Found 1 hazards 200 unsafe references 0 missing
[task 2020-04-21T21:29:56.614Z] Running heapwrites to generate heapWriteHazards.txt
[task 2020-04-21T21:29:56.614Z] PATH="/builds/worker/fetches/sixgill/usr/bin:${PATH}" LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed' SOURCE='/builds/worker/checkouts/gecko' XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' /builds/worker/workspace/obj-haz-shell/dist/bin/js /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyzeHeapWrites.js > heapWriteHazards.txt
[task 2020-04-21T21:30:26.492Z] + check_hazards /builds/worker/workspace/analysis
[task 2020-04-21T21:30:26.493Z] + set +e
[task 2020-04-21T21:30:26.493Z] ++ grep -c 'Function.*has unrooted.*live across GC call' /builds/worker/workspace/analysis/rootingHazards.txt
[task 2020-04-21T21:30:26.496Z] + NUM_HAZARDS=1
[task 2020-04-21T21:30:26.496Z] ++ grep -c '^Function.*takes unsafe address of unrooted' /builds/worker/workspace/analysis/refs.txt
[task 2020-04-21T21:30:26.497Z] + NUM_UNSAFE=200
[task 2020-04-21T21:30:26.497Z] ++ grep -c '^Function.* has unnecessary root' /builds/worker/workspace/analysis/unnecessary.txt
[task 2020-04-21T21:30:26.499Z] + NUM_UNNECESSARY=1268
[task 2020-04-21T21:30:26.499Z] ++ grep -c '^Dropped CFG' /builds/worker/workspace/analysis/build_xgill.log
[task 2020-04-21T21:30:26.630Z] + NUM_DROPPED=0
[task 2020-04-21T21:30:26.631Z] ++ perl -lne 'print $1 if m!found (\d+)/\d+ allowed errors!' /builds/worker/workspace/analysis/heapWriteHazards.txt
[task 2020-04-21T21:30:26.647Z] + NUM_WRITE_HAZARDS=0
[task 2020-04-21T21:30:26.647Z] ++ grep -c '^Function.*expected hazard.*but none were found' /builds/worker/workspace/analysis/rootingHazards.txt
[task 2020-04-21T21:30:26.648Z] + NUM_MISSING=0
[task 2020-04-21T21:30:26.648Z] + set +x
[task 2020-04-21T21:30:26.648Z] TinderboxPrint: rooting hazards<br/>1
[task 2020-04-21T21:30:26.648Z] TinderboxPrint: (unsafe references to unrooted GC pointers)<br/>200
[task 2020-04-21T21:30:26.648Z] TinderboxPrint: (unnecessary roots)<br/>1268
[task 2020-04-21T21:30:26.648Z] TinderboxPrint: missing expected hazards<br/>0
[task 2020-04-21T21:30:26.648Z] TinderboxPrint: heap write hazards<br/>0
[task 2020-04-21T21:30:26.649Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'response' of type 'JS::Value' live across GC call at dom/indexedDB/ActorsChild.cpp:2711
[task 2020-04-21T21:30:26.649Z] TEST-UNEXPECTED-FAIL | hazards | 1 rooting hazards detected
[task 2020-04-21T21:30:26.649Z] TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_rooting_hazards_failure'>static rooting hazard analysis failures</a>, visit "Inspect Task" link for hazard details
[task 2020-04-21T21:30:26.650Z] + onexit

Comment 1

[Bogdan Tara[:bogdan_tara | bogdant]]

sg, this seems related to the changes from bug 1618542 and bug 1623278.

Could you, please, take a look?

Comment 2

[Simon Giesecke [:sg] [he/him]]

Hm, what changed is where the transaction is addrefed (and possibly be deleted, but this can never happen here). But I wonder if the function should use JS::Rooted somehow:

void BackgroundRequestChild::HandleResponse(uint64_t aResponse) {
  AssertIsOnOwningThread();

  JS::Rooted<JS::Value> response(/* where do we get a JS Context? */, JS::NumberValue(aResponse));
  // was:
 //  JS::Value response(JS::NumberValue(aResponse));

   // something needs to be changed here or in ResultHelper as well then:
  ResultHelper helper(mRequest, AcquireTransaction(), &response);

  helper.DispatchSuccessEvent();
}```

Comment 3

[Intermittent Failures Robot]

54 failures in 1127 pushes (0.048 failures/push) were associated with this bug yesterday.

Repository breakdown:

• autoland: 54

Platform breakdown:

• linux64: 4
• windows7-32: 4
• windows10-64: 9
• linux1804-64: 1
• macosx1014-64: 10
• linux1804-64-asan: 25
• windows10-64-mingwclang: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-04-22&endday=2020-04-22&tree=all

Comment 4

[Intermittent Failures Robot]

75 failures in 5582 pushes (0.013 failures/push) were associated with this bug in the last 7 days.

This is the #24 most frequent failure this week.

** This failure happened more than 75 times this week! Resolving this bug is a very high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 1 week, the affected test(s) may be disabled. **

Repository breakdown:

• autoland: 73
• try: 2

Platform breakdown:

• linux64: 20
• windows7-32: 6
• windows10-64: 10
• linux1804-64: 2
• macosx1014-64: 11
• linux1804-64-asan: 25
• windows10-64-mingwclang: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-04-20&endday=2020-04-26&tree=all

Comment 5

[Stefan Hindli [:stefan_hindli]]

Over the last 7 days there are 35 failures on this bug. These happen on: windows7-32, windows10-64, macosx1014-64, linux64
Here is the latest failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=300137352&repo=autoland&lineNumber=71801

Comment 6

[Intermittent Failures Robot]

25 failures in 5423 pushes (0.005 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 3
• autoland: 21
• mozilla-central: 1

Platform breakdown:

• linux64: 25

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-04-27&endday=2020-05-03&tree=all

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

In the last 7 days there have been 26 occurrences, all on linux64 debug.

Recent failure: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=301518361&repo=autoland&lineNumber=76977

Comment 8

[Intermittent Failures Robot]

27 failures in 5276 pushes (0.005 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 27

Platform breakdown:

• linux64: 27

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-05-04&endday=2020-05-10&tree=all

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Bug 1631970 - Delay converting uint64 results to JSVals. r=sg

The rooting hazard analysis is getting upset because of the usage where
a non-GCThing JSVal is declared on the stack and AcquireTransaction() is
called. The analysis doesn't know that it's guaranteed that the value is
NaN-boxed and can't be a GC thing. It also doesn't know that the SafeRefPtr
can't already have a value so a release is impossible.

Arguably the hazard analysis doesn't need to know these things though.

This fix delays the point at which we convert the uint64 into a JS::Value.
The call to GetResult
already has a rooted result and the (Auto)JSAPI initialized, so this is
arguably cleaner and less scary in terms of holding raw pointers in the
ResultHelper that aren't necessary.

This fix doesn't correct the ResultTypeJSValHandle which is a similar
situation, except it's a reference to
UndefinedHandleValue
which is I suppose more fine from a GC perspective. Still somewhat ugly
as used in ResultHelper, but I'm presuming we'll clean ResultHelper up
in the future to use a Variant, at which point we can explicitly indicate
we want undefined.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e96931a667e5
Delay converting uint64 results to JSVals. r=sg

Comment 11

[Andreea Pavel [:apavel]]

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

Comment 12

[Intermittent Failures Robot]

12 failures in 5488 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 12

Platform breakdown:

• linux64: 12

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-05-11&endday=2020-05-17&tree=all

Comment 13

[Intermittent Failures Robot]

1 failures in 4727 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 1

Platform breakdown:

• linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-05-18&endday=2020-05-24&tree=all

Comment 14

[Intermittent Failures Robot]

2 failures in 5458 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-release: 1
• mozilla-beta: 1

Platform breakdown:

• linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1631970&startday=2020-05-25&endday=2020-05-31&tree=all