Closed Bug 1351246 Opened 3 years ago Closed 3 years ago

(xpcshell:39978): Gdk-CRITICAL **: gdk_screen_get_root_window: assertion `GDK_IS_SCREEN (screen)' failed

Categories

(Core :: Widget: Gtk, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: jbeich, Assigned: kanru)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

$ c++ -v
FreeBSD clang version 4.0.0 (tags/RELEASE_400/final 297347) (based on LLVM 4.0.0)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin

$ pkg info -x \^gtk \^glib
gtk-update-icon-cache-2.24.29
gtk2-2.24.29_3
gtk3-3.18.8_3
glib-2.46.2_4

$ ./mach bootstrap
$ ./mach build
$ ./mach package
[...]
resource://gre/modules/sdk/bootstrap.js
resource://gre/modules/sdk/system/Startup.js
Traceback (most recent call last):
  File "toolkit/mozapps/installer/packager.py", line 397, in <module>
    main()
  File "toolkit/mozapps/installer/packager.py", line 391, in main
    args.source, gre_path, base)
  File "toolkit/mozapps/installer/packager.py", line 165, in precompile_cache
    errors.fatal('Error while running startup cache precompilation')
  File "python/mozbuild/mozpack/errors.py", line 103, in fatal
    self._handle(self.FATAL, msg)
  File "python/mozbuild/mozpack/errors.py", line 98, in _handle
    raise ErrorMessage(msg)
mozpack.errors.ErrorMessage: Error: Error while running startup cache precompilation
gmake[3]: *** [toolkit/mozapps/installer/packager.mk:41: stage-package] Error 1
gmake[3]: Leaving directory 'obj-x86_64-unknown-freebsd12.0/browser/installer'
[...]
resource://gre/modules/sdk/bootstrap.js
resource://gre/modules/sdk/system/Startup.js
Process 9405 stopped
* thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: libgdk-3.so.0`gdk_screen_get_root_window(screen=0x0000000000000000) at gdkscreen.c:695
   692  {
   693    g_return_val_if_fail (GDK_IS_SCREEN (screen), NULL);
   694
-> 695    return GDK_SCREEN_GET_CLASS (screen)->get_root_window (screen);
   696  }
   697
   698  /**
(lldb) bt
* thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
  * frame #0: libgdk-3.so.0`gdk_screen_get_root_window(screen=0x0000000000000000) at gdkscreen.c:695
    frame #1: libgdk-3.so.0`gdk_get_default_root_window at gdkwindow.c:4798
    frame #2: libxul.so`mozilla::widget::ScreenHelperGTK::ScreenHelperGTK(this=0x000000081a99afa0) at ScreenHelperGTK.cpp:79
    frame #3: libxul.so`nsAppShell::Init(void) [inlined] _ZN7mozilla10MakeUniqueINS_6widget15ScreenHelperGTKEJEEENS_6detail14UniqueSelectorIT_E12SingleObjectEDpOT0_ at UniquePtr.h:680
    frame #4: libxul.so`nsAppShell::Init(this=0x000000081a940e00) at nsAppShell.cpp:173
    frame #5: libxul.so`nsAppShellInit(void) at nsAppShellSingleton.h:45
    frame #6: libxul.so`nsFactoryEntry::GetFactory(void) at nsComponentManager.cpp:814
    frame #7: libxul.so`nsFactoryEntry::GetFactory(this=0x00000008114f8c20) at nsComponentManager.cpp:1836
    frame #8: libxul.so`nsComponentManagerImpl::CreateInstance(this=0x00000008114943c0, aClass=<unavailable>, aDelegate=0x0000000000000000, aIID=<unavailable>, aResult=0x00007fffffffbb10) at nsComponentManager.cpp:1055
    frame #9: libxul.so`nsComponentManagerImpl::GetService(this=0x00000008114943c0, aClass=0x0000000805617de8, aIID=<unavailable>, aResult=0x00007fffffffbb80) at nsComponentManager.cpp:1300
    frame #10: libxul.so`nsGetServiceByCIDWithError::operator()(nsID const&, void**) const [inlined]CallGetService(aCID=<unavailable>) at nsComponentManagerUtils.cpp:56
    frame #11: libxul.so`nsGetServiceByCIDWithError::operator(this=0x00007fffffffbba8, aIID=<unavailable>, aInstancePtr=0x00007fffffffbb80)(nsID const&, void**) const at nsComponentManagerUtils.cpp:265
    frame #12: libxul.so`nsCOMPtr_base::assign_from_gs_cid_with_error(this=0x0000000819ecedc0, aGS=<unavailable>, aIID=<unavailable>) at nsCOMPtr.cpp:84
    frame #13: libxul.so`nsAppStartup::Init(void) [inlined] nsCOMPtr<nsIAppShell>::operator=(aRhs=<unavailable>) at nsCOMPtr.h:652
    frame #14: libxul.so`nsAppStartup::Init(this=0x0000000819eced90) at nsAppStartup.cpp:158
    frame #15: libxul.so`nsAppStartupConstructor(aOuter=<unavailable>, aIID=0x000000081158df50, aResult=0x00007fffffffbc70) at nsToolkitCompsModule.cpp:65
    frame #16: libxul.so`nsComponentManagerImpl::CreateInstance(this=<unavailable>, aClass=<unavailable>, aDelegate=0x0000000000000000, aIID=<unavailable>, aResult=0x00007fffffffbc70) at nsComponentManager.cpp:1057
    frame #17: libxul.so`nsComponentManagerImpl::GetService(this=0x00000008114943c0, aClass=0x000000081a94a630, aIID=<unavailable>, aResult=0x00007fffffffbcf0) at nsComponentManager.cpp:1300
    frame #18: libxul.so`JSContext* nsJSCID::GetService(this=0x000000081a99aea0, iidval=<unavailable>, cx=<unavailable>, optionalArgc=<unavailable>, retval=<unavailable>) at XPCJSID.cpp:695
    frame #19: libxul.so`NS_InvokeByIndex at xptcinvoke_asm_x86_64_unix.S:115
    frame #20: libxul.so`XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [inlined] CallMethodHelper::Invoke(void) at XPCWrappedNative.cpp:2010
    frame #21: libxul.so`XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [inlined] CallMethodHelper::Call(void) at XPCWrappedNative.cpp:1329
    frame #22: libxul.so`XPCWrappedNative::CallMethod(ccx=<unavailable>, mode=<unavailable>) at XPCWrappedNative.cpp:1296
    frame #23: libxul.so`XPC_WN_CallMethod(cx=<unavailable>, argc=1, vp=0x0000000814efd288) at XPCWrappedNativeJSOps.cpp:983
    frame #24: libxul.so`js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) [inlined] js::CallJSNative(native=(libxul.so`XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) at XPCWrappedNativeJSOps.cpp:962))(JSContextunsigned intJS::Value*), JSContext*::CallArgs const&) at jscntxtinlines.h:282
    frame #25: libxul.so`js::InternalCallOrConstruct(cx=0x0000000811422800, args=0x00007fffffffc350,construct=<unavailable>) at Interpreter.cpp:454
    frame #26: libxul.so`Interpret(JSContext*, js::RunState&) [inlined] js::CallFromStack(cx=0x0000000811422800) at Interpreter.cpp:505
    frame #27: libxul.so`Interpret(cx=<unavailable>, state=<unavailable>) at Interpreter.cpp:2998
    frame #28: libxul.so`js::RunScript(cx=0x0000000811422800, state=0x00007fffffffc6a0) at Interpreter.cpp:394
    frame #29: libxul.so`JSObject& js::ExecuteKernel(cx=<unavailable>, script=<unavailable>, envChainArg=<unavailable>, newTargetValue=<unavailable>, evalInFrame=<unavailable>, result=<unavailable>) atInterpreter.cpp:683
    frame #30: libxul.so`JSObject& js::Execute(cx=0x0000000811422800, script=<unavailable>, envChainArg=<unavailable>, rval=0x0000000000000000) at Interpreter.cpp:715
    frame #31: libxul.so`JS_ExecuteScript(cx=0x0000000811422800, scriptArg=<unavailable>) at jsapi.cpp:4539
    frame #32: libxul.so`JS<JSScript*> mozJSComponentLoader::ObjectForLocation(this=0x00000008115d4900, aInfo=<unavailable>, aComponentFile=0x000000081a94a530, aObject=<unavailable>, aTableScript=<unavailable>, aLocation=0x000000081a8d3128, aPropagateExceptions=<unavailable>, aException=<unavailable>)at mozJSComponentLoader.cpp:969
    frame #33: libxul.so`JSContext* mozJSComponentLoader::ImportInto(this=<unavailable>, aLocation=<unavailable>, targetObj=<unavailable>, callercx=<unavailable>, vp=<unavailable>) at mozJSComponentLoader.cpp:1200
    frame #34: libxul.so`JSContext* mozJSComponentLoader::Import(this=<unavailable>, registryLocation=<unavailable>, targetValArg=<unavailable>, cx=0x0000000811422800, optionalArgc=<unavailable>, retval=<unavailable>) at mozJSComponentLoader.cpp:1076
    frame #35: libxul.so`JSContext* nsXPCComponents_Utils::Import(this=<unavailable>, registryLocation=0x000000081146c030, targetObj=JS::HandleValue @ r12, cx=0x0000000811422800, optionalArgc='\x01', retval=JS::MutableHandleValue @ r14) at XPCComponents.cpp:2504
    frame #36: libxul.so`NS_InvokeByIndex at xptcinvoke_asm_x86_64_unix.S:115
    frame #37: libxul.so`XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [inlined] CallMethodHelper::Invoke(void) at XPCWrappedNative.cpp:2010
    frame #38: libxul.so`XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [inlined] CallMethodHelper::Call(void) at XPCWrappedNative.cpp:1329
    frame #39: libxul.so`XPCWrappedNative::CallMethod(ccx=<unavailable>, mode=<unavailable>) at XPCWrappedNative.cpp:1296
    frame #40: libxul.so`XPC_WN_CallMethod(cx=<unavailable>, argc=2, vp=0x00007fffffffd238) at XPCWrappedNativeJSOps.cpp:983
    frame #41: 0x000019050efea2c4
    frame #42: 0x0000000814fc49c0
    frame #43: 0x000019050efd58a6
    frame #44: libxul.so`EnterBaseline(cx=0x000000081219e2e0, data=0xfffe00081219e2e0) at BaselineJIT.cpp:160
    frame #45: libxul.so`js::jit::EnterBaselineAtBranch(cx=0x0000000811422800, fp=0x0000000814efd140, pc=<unavailable>) at BaselineJIT.cpp:268
    frame #46: libxul.so`Interpret(cx=<unavailable>, state=<unavailable>) at Interpreter.cpp:1963
    frame #47: libxul.so`js::RunScript(cx=0x0000000811422800, state=0x00007fffffffdb60) at Interpreter.cpp:394
    frame #48: libxul.so`JSObject& js::ExecuteKernel(cx=<unavailable>, script=<unavailable>, envChainArg=<unavailable>, newTargetValue=<unavailable>, evalInFrame=<unavailable>, result=<unavailable>) atInterpreter.cpp:683
    frame #49: libxul.so`JSObject& js::Execute(cx=0x0000000811422800, script=<unavailable>, envChainArg=<unavailable>, rval=0x00007fffffffe0a0) at Interpreter.cpp:715
    frame #50: libxul.so`JS::Handle::ReadOnlyCompileOptions const& Evaluate(cx=0x0000000811422800, scopeKind=Global, env=<unavailable>, optionsArg=0x00007fffffffdfd0, srcBuf=0x00007fffffffdd80, rval=<unavailable>) at jsapi.cpp:4595
    frame #51: libxul.so`JS::Evaluate(cx=0x0000000811422800, options=0x00007fffffffdfd0, bytes=<unavailable>, length=<unavailable>, rval=JS::MutableHandleValue @ r14) at jsapi.cpp:4648
    frame #52: libxul.so`XRE_XPCShellMain(int, char**, char**, XREShellData const*) at XPCShellImpl.cpp:1139
    frame #53: libxul.so`XRE_XPCShellMain(argc=<unavailable>, argv=<unavailable>, envp=<unavailable>, aShellData=<unavailable>) at XPCShellImpl.cpp:1612
    frame #54: xpcshell`main(argc=<unavailable>, argv=<unavailable>, envp=0x00007fffffffe2b8) at xpcshell.cpp:68
    frame #55: xpcshell`_start(ap=<unavailable>, cleanup=<unavailable>) at crt1.c:72
(lldb) f 2
frame #2: libxul.so`mozilla::widget::ScreenHelperGTK::ScreenHelperGTK(this=0x000000081a99afa0) at ScreenHelperGTK.cpp:79
   76     , mNetWorkareaAtom(0)
   77   {
   78     MOZ_LOG(sScreenLog, LogLevel::Debug, ("ScreenHelperGTK created"));
-> 79     mRootWindow = gdk_get_default_root_window();
   80     if (!mRootWindow) {
   81       // Sometimes we don't initial X (e.g., xpcshell)
   82       MOZ_LOG(sScreenLog, LogLevel::Debug, ("mRootWindow is nullptr, running headless"));
(lldb) fr v
(mozilla::widget::ScreenHelperGTK *) this = 0x000000081a99afa0
(lldb) p *this
(mozilla::widget::ScreenHelperGTK) $1 = {
  mXineramalib = 0x0000000000000000
  mRootWindow = 0x0000000000000000
  mNetWorkareaAtom = 0
}
(lldb) f 4
frame #4: libxul.so`nsAppShell::Init(this=0x000000081a940e00) at nsAppShell.cpp:173
   170
   171      if (XRE_IsParentProcess()) {
   172          ScreenManager& screenManager = ScreenManager::GetSingleton();
-> 173          screenManager.SetHelper(mozilla::MakeUnique<ScreenHelperGTK>());
   174      }
   175
   176  #if MOZ_WIDGET_GTK == 3
mozilla-central changeset 4be4367d022d is the first bad commit
Is it FreeBSD specific? Are you running |./mach package| without X or headless? I cannot reproduce locally.

Karl, do you know what should I check to avoid this?
Flags: needinfo?(karlt)
Flags: needinfo?(jbeich)
(In reply to Jan Beich from comment #0)
> -> 79     mRootWindow = gdk_get_default_root_window();
>    80     if (!mRootWindow) {
>    81       // Sometimes we don't initial X (e.g., xpcshell)

Checking gdk_screen_get_default() appears to fix the crash.

(In reply to Kan-Ru Chen [:kanru] (UTC-4, EDT) from comment #2)
> Is it FreeBSD specific?

I'm not sure. It could be specific to my environment as well.

> Are you running |./mach package| without X or headless? I cannot reproduce locally.

No, I'm building from X11-connected terminal. DISPLAY is defined in environment.
Flags: needinfo?(jbeich)
g_return_val_if_fail() is for programming errors only.  It may not be a no-op if libgtk is configured with non-default options.

The programming error here may be in the design of gdk_get_default_root_window().

The workaround is to use gdk_screen_get_default() and gdk_screen_get_root_window().
The result of only the former need be null-checked.

gdk_screen_get_default() is already called in this method.

(Another thing this highlights is that the screen manager is now initialized on start-up, instead of lazily when required, but I don't know whether there is a good way to change that.)
Flags: needinfo?(karlt)
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

https://reviewboard.mozilla.org/r/124674/#review127418
Attachment #8852442 - Flags: review?(karlt) → review+
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

https://reviewboard.mozilla.org/r/124674/#review127496

::: widget/gtk/ScreenHelperGTK.cpp:85
(Diff revision 1)
> -  if (!mRootWindow) {
> +  if (!defaultScreen) {
>      // Sometimes we don't initial X (e.g., xpcshell)
>      MOZ_LOG(sScreenLog, LogLevel::Debug, ("mRootWindow is nullptr, running headless"));
>      return;
>    }
> +  mRootWindow = gdk_get_default_root_window();

Maybe replace `gdk_get_default_root_window()` with `gdk_screen_get_root_window(defaultScreen)` to save an extra call to `gdk_screen_get_default()` from within Gtk.
(In reply to Karl Tomlinson (:karlt) from comment #5)
> g_return_val_if_fail() is for programming errors only.  It may not be a
> no-op if libgtk is configured with non-default options.

Gtk+ passes -DG_DISABLE_CHECKS for --disable-debug builds which turns off g_return_val_if_fail() to avoid scaring users with warnings like:

  (xpcshell:73648): Gdk-CRITICAL **: gdk_screen_get_root_window: assertion 'GDK_IS_SCREEN (screen)' failed

https://git.gnome.org/browse/gtk+/diff/configure.in?id=fc7e845c8f

./mach packages shows a few more that maybe unrelated:

  (xpcshell:73648): GLib-GObject-WARNING **: invalid (NULL) pointer instance
  (xpcshell:73648): GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion'G_TYPE_CHECK_INSTANCE (instance)' failed
Summary: ./mach package (or ./mach install) crashes in nsAppShell::Init → (xpcshell:39978): Gdk-CRITICAL **: gdk_screen_get_root_window: assertion `GDK_IS_SCREEN (screen)' failed
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

https://reviewboard.mozilla.org/r/124674/#review127506

::: widget/gtk/ScreenHelperGTK.cpp:109
(Diff revision 1)
>    RefreshScreens();
>  }
>  
>  ScreenHelperGTK::~ScreenHelperGTK()
>  {
>    g_signal_handlers_disconnect_by_func(gdk_screen_get_default(),

Can you add a similar check here for the 2nd assert in comment 9?
(In reply to Karl Tomlinson (:karlt) from comment #5)
> It may not be a no-op if libgtk is configured with non-default options.

Too many negatives there, sorry.  I meant it may be a no-op if libgtk is configured with non-default options.
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

https://reviewboard.mozilla.org/r/124674/#review127506

> Can you add a similar check here for the 2nd assert in comment 9?

Thanks.  Better to move it to the if (mRootWindow) block.
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

https://reviewboard.mozilla.org/r/124674/#review127496

> Maybe replace `gdk_get_default_root_window()` with `gdk_screen_get_root_window(defaultScreen)` to save an extra call to `gdk_screen_get_default()` from within Gtk.

Yes, that was what I was thinking, but gdk_get_default_root_window() save passing one parameter, so I don't mind which.
(In reply to Jan Beich from comment #9)
> (In reply to Karl Tomlinson (:karlt) from comment #5)
> > g_return_val_if_fail() is for programming errors only.  It may not be a
> > no-op if libgtk is configured with non-default options.
> 
> Gtk+ passes -DG_DISABLE_CHECKS for --disable-debug builds which turns off
> g_return_val_if_fail() to avoid scaring users with warnings like:
> 
>   (xpcshell:73648): Gdk-CRITICAL **: gdk_screen_get_root_window: assertion
> 'GDK_IS_SCREEN (screen)' failed

Those are usually there to protect against the subsequent code going wrong, and so instead of having scared users, users will be confused about broken apps with no messages.
Comment on attachment 8852442 [details]
Bug 1351246 - Check gdk_screen_get_default() for X.

Thank you. With the patch applied "./mach package" succeeds without a crash or gtk/glib assertions here.
Attachment #8852442 - Flags: feedback+
Assignee: nobody → kchen
Pushed by kchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0fea86135263
Check gdk_screen_get_default() for X. r=karlt
https://hg.mozilla.org/mozilla-central/rev/0fea86135263
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
See Also: → 1388810
You need to log in before you can comment on or make changes to this bug.