1153173 - Uninitialised value use in AutoJSExceptionReporter::~AutoJSExceptionReporter

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Julian Seward [:jseward]]

It appears that ~AutoJSExceptionReporter can check its mIsDestroyPending
field with that ever having been initialised.  Looking at the constructor
AutoJSExceptionReporter::AutoJSExceptionReporter, mIsDestroyPending is
simply copied from nsJSObjWrapper::mDestroyPending.  It may be that the
root error is that nsJSObjWrapper::nsJSObjWrapper doesn't set mDestroyPending.

Possibly related to bug 1135491.

Comment 1

[Julian Seward [:jseward]]

STR: test path is dom/base/test/csp/test_CSP.html

It's pretty easy to demonstrate, though, happens on quite a lot
of runs.

I ran this:

DISPLAY=:1.0 G_SLICE=always-malloc MOZ_DISABLE_NONLOCAL_CONNECTIONS=0 \
  ./mach mochitest-plain --e10s \
  --debugger=/home/sewardj/VgTRUNK/mozhx/Inst/bin/valgrind \
  --debugger-args="--fair-sched=yes --smc-check=all-non-file \
    --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no \
    --trace-children=yes --child-silent-after-fork=yes \
    --trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep,*/bin/java \
    --num-transtab-sectors=24 --tool=memcheck --freelist-vol=500000000 \
    --redzone-size=256 --gen-suppressions=no \
    --px-default=allregs-at-mem-access \
    --px-file-backed=unwindregs-at-mem-access --stats=yes \
    --partial-loads-ok=yes --show-mismatched-frees=no --read-inline-info=yes \
    --fullpath-after=-CURR/ --num-callers=16 --track-origins=yes" \
  dom/base/test/csp/test_CSP.html 2>&1 | tee logfile-05-mc

Comment 2

[Julian Seward [:jseward]]

Attachment: Valgrind complainage

Comment 3

[Julian Seward [:jseward]]

Attachment: bug1153173-1.diff

A simple fix.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Review of attachment 8590757 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for catching this!

Comment 5

[Julian Seward [:jseward]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce79d0eb8636

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Approval Request Comment
[Feature/regressing bug #]: bug 1135491
[User impact if declined]: Possibility of incorrect behavior of scripted plugin objects.
[Describe test coverage new/current, TreeHerder]: Caught by dom/base/test/csp/test_CSP.html + Valgrind
[Risks and why]: None, trivial patch that fixes an uninitialized variable
[String/UUID change made/needed]: None

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #6)
> [Feature/regressing bug #]: bug 1135491

Bug 1135491 was fixed in 38. Do we need to take the fix in 38 as well?

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Aurora+

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

Good point. I'd recommend that we do.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Approval Request Comment
[Feature/regressing bug #]: bug 1135491
[User impact if declined]: Possibility of incorrect behavior of scripted plugin objects.
[Describe test coverage new/current, TreeHerder]: Caught by dom/base/test/csp/test_CSP.html + Valgrind
[Risks and why]: None, trivial patch that fixes an uninitialized variable
[String/UUID change made/needed]: None

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

Should be in 38 beta 7.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8590757 [details] [diff] [review]
bug1153173-1.diff

[Triage Comment]
uplift to m-r as we did the 38 merge already

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22868e71c861

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/92fb098ace7a

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/92fb098ace7a