1968312 - Null piWindow crash in [@ ProfileResetCleanup]

Status: NEW

(Toolkit :: Startup and Profile System, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/76f987a8-4d69-4e04-b381-615760250521

Reason:

EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames:

0  XUL  ProfileResetCleanup(nsToolkitProfileService*, nsIToolkitProfile*)  toolkit/xre/ProfileReset.cpp:143
1  XUL  XREMain::XRE_mainRun()  toolkit/xre/nsAppRunner.cpp:5600
2  XUL  XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6138
3  XUL  XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6211
4  firefox  do_main(int, char**, char**)  browser/app/nsBrowserApp.cpp:232
4  firefox  main  browser/app/nsBrowserApp.cpp:464
5  dyld  start
6  firefox  _mh_execute_header
7  dyld  __Block_byref_object_dispose_.12
8  libmozglue.dylib  double_conversion::DoubleToStringConverter::HandleSpecialValues(double, doubl...  mfbt/double-conversion/double-conversion/double-to-string.cc:72

48 crashes with this signature in the last 6 months, which isn't a very high volume. The ones I looked at were all on the piWindow call here:

  // Close the progress window now that the cleanup thread is done.
  auto* piWindow = nsPIDOMWindowOuter::From(progressWindow);
  piWindow->Close();

Seems like there should just be a null check on piWindow? It does seem a little weird that OpenWindow doesn't fail but still returns null.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Moving to "Toolkit :: Startup and Profile System" (based on the fact that toolkit/xre/ProfileReset.cpp where the crash is being hit is reported to be part of that bugzilla component).

Comment 2

[Dave Townsend [:mossop]]

(In reply to Andrew McCreight [:mccr8] from comment #0)

Crash report: https://crash-stats.mozilla.org/report/index/76f987a8-4d69-4e04-b381-615760250521

Reason:

EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames:

0  XUL  ProfileResetCleanup(nsToolkitProfileService*, nsIToolkitProfile*)  toolkit/xre/ProfileReset.cpp:143
1  XUL  XREMain::XRE_mainRun()  toolkit/xre/nsAppRunner.cpp:5600
2  XUL  XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6138
3  XUL  XRE_main(int, char**, mozilla::BootstrapConfig const&)  toolkit/xre/nsAppRunner.cpp:6211
4  firefox  do_main(int, char**, char**)  browser/app/nsBrowserApp.cpp:232
4  firefox  main  browser/app/nsBrowserApp.cpp:464
5  dyld  start
6  firefox  _mh_execute_header
7  dyld  __Block_byref_object_dispose_.12
8  libmozglue.dylib  double_conversion::DoubleToStringConverter::HandleSpecialValues(double, doubl...  mfbt/double-conversion/double-conversion/double-to-string.cc:72

48 crashes with this signature in the last 6 months, which isn't a very high volume. The ones I looked at were all on the piWindow call here:

  // Close the progress window now that the cleanup thread is done.
  auto* piWindow = nsPIDOMWindowOuter::From(progressWindow);
  piWindow->Close();

Seems like there should just be a null check on piWindow? It does seem a little weird that OpenWindow doesn't fail but still returns null.

It appears that in at least some cases OpenWindow will return NS_OK but a null window in the event that the window open is immediately cancelled. I'm not sure how that could be happening here, but a null check seems fine.