1181960 - Need to call gdk_threads_init before gtk_init

Status: RESOLVED INVALID

(Core :: Widget: Gtk, defect)

Description

[noitidart]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150706172413

Steps to reproduce:

Writing GTK+ js-ctypes in ChromeWorker's from addons. 

When using UI functions in GTK2 apps and multithreading (ChromeWorker) i have to surround my UI function such as gdk_pixbuf_get_from_drawable with gdk_threads_enter() and gdk_threads_leave() otherwise firefox crashes.

I cant do enter/leave though unless gdk_threads_init() was called, but docs say this must be done before gtk_init().


Actual results:

I tried doing gdk_threads_init() from ChromeWorker, then i can do gdk_threads_enter() my UI function and gdk_threads_leave() however eventually firefox crashes.


Expected results:

Do gdk_threads_init() before gtk_init() so worker threads can use enter/leave

Comment 1

[noitidart]

Why this bug came up:

1) Is there a reason why firefox doesnt call gtk_threads_init before gtk_init?
2) because now addons doing ctypes from workers are having problems
3) if they run gtk_threads_init, then someone else runs it then it will be a multiple run and the multiple runs seems to be messing things up 
4) becasue from our workers we are having to use gdk_threads_enter() then function then gdk_threads_leave() and we cant uses these enter/leave func unless gdk_threads_init was called by mainthread or some thread
5) and i think our addons still crash because gdk_threads_init needs to be called by mainthread before gtk_init 

Based on this article here: http://blogs.operationaldynamics.com/andrew/software/gnome-desktop/gtk-thread-awareness

Comment 2

[:Gijs (he/him)]

This seems like something that would have been better off as a newsgroup question than a bug, but fine...

Karl, do you have ideas about whether we could do anything differently here?

Comment 3

[noitidart]

Thanks @gijs, using g_idle_callback is not working either, its crashing even though people in multiple places are saying it should work: http://stackoverflow.com/questions/31306189/gtk2-multi-thread-gdk-pixbuf-get-from-drawable-questions/31306545?noredirect=1#comment50650916_31306545

Comment 4

[Karl Tomlinson (:karlt)]

(In reply to noitidart from comment #1)
> Why this bug came up:
> 
> 1) Is there a reason why firefox doesnt call gtk_threads_init before
> gtk_init?

gtk_threads_init is only part of what is required to use GDK threads.
I expect enter/leave would be needed in a number of other places also.

This hasn't been necessary and would be pointless at this stage because these
methods are deprecated in recent GDK 3.

> 2) because now addons doing ctypes from workers are having problems

This hasn't changed.  gtk_threads_init has never been called AFAIK, at least
not intentionally.

I suggest using Xlib instead of GDK, if this needs to be on a worker thread.

(In reply to noitidart from comment #3)
> Thanks @gijs, using g_idle_callback is not working either, its crashing even
> though people in multiple places are saying it should work:
> http://stackoverflow.com/questions/31306189/gtk2-multi-thread-gdk-pixbuf-get-
> from-drawable-questions/31306545?noredirect=1#comment50650916_31306545

GLib thread functions can be used on multiple threads.

Comment 5

[noitidart]

(In reply to Karl Tomlinson (ni?:karlt) from comment #4)
> (In reply to noitidart from comment #1)
> > Why this bug came up:
> > 
> > 1) Is there a reason why firefox doesnt call gtk_threads_init before
> > gtk_init?
> 
> gtk_threads_init is only part of what is required to use GDK threads.
> I expect enter/leave would be needed in a number of other places also.
> 
> This hasn't been necessary and would be pointless at this stage because these
> methods are deprecated in recent GDK 3.
> 
> > 2) because now addons doing ctypes from workers are having problems
> 
> This hasn't changed.  gtk_threads_init has never been called AFAIK, at least
> not intentionally.
> 
> I suggest using Xlib instead of GDK, if this needs to be on a worker thread.
> 
> (In reply to noitidart from comment #3)
> > Thanks @gijs, using g_idle_callback is not working either, its crashing even
> > though people in multiple places are saying it should work:
> > http://stackoverflow.com/questions/31306189/gtk2-multi-thread-gdk-pixbuf-get-
> > from-drawable-questions/31306545?noredirect=1#comment50650916_31306545
> 
> GLib thread functions can be used on multiple threads.

Thanks Karl, by GLib thread functions do you mean these g_idle_add? Are you sure? Because me and a friend spent hours debugging it with gdb and here are the results, the readme tracks it very carefully, we found that whenever we try to run a function with g_idle_callback it crashes and the gdb output is here: https://gist.github.com/Noitidart/a8e7c81a02b0f60c75a9#comment-1488145

It seemed not possible to run from othe thread. I ended up having to post back to the main thread, others working on gtk with jsctypes in threads are encountering the same issues.

This is a simple cross platform native screenshot addon im working on, this bugzilla topic isnt meant for me to ask help on this but point out the crashing with the work. This here doesnt crash because it runs the function on the main thread: https://github.com/Noitidart/NativeShot/blob/master/modules/workers/MainWorker.js#L453-L606

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to noitidart from comment #5)
> Thanks Karl, by GLib thread functions do you mean these g_idle_add? Are you
> sure? Because me and a friend spent hours debugging it with gdb and here are
> the results, the readme tracks it very carefully, we found that whenever we
> try to run a function with g_idle_callback it crashes and the gdb output is
> here: https://gist.github.com/Noitidart/a8e7c81a02b0f60c75a9#comment-1488145

Most GLib functions, including g_idle_add I assume, can run on any thread.

That crash is from a Gecko method.  It looks like an assertion failure because a particular Gecko object or method is not safe to use off main thread.

A libxul.so with symbols (not stripped) would be the best way to find the particular method involved, or maybe you know from the js.

Comment 7

[noitidart]

(In reply to Karl Tomlinson (ni?:karlt) from comment #6)
> (In reply to noitidart from comment #5)
> > Thanks Karl, by GLib thread functions do you mean these g_idle_add? Are you
> > sure? Because me and a friend spent hours debugging it with gdb and here are
> > the results, the readme tracks it very carefully, we found that whenever we
> > try to run a function with g_idle_callback it crashes and the gdb output is
> > here: https://gist.github.com/Noitidart/a8e7c81a02b0f60c75a9#comment-1488145
> 
> Most GLib functions, including g_idle_add I assume, can run on any thread.
> 
> That crash is from a Gecko method.  It looks like an assertion failure
> because a particular Gecko object or method is not safe to use off main
> thread.
> 
> A libxul.so with symbols (not stripped) would be the best way to find the
> particular method involved, or maybe you know from the js.


Thanks Karl for such a fast reply. Yeah me and my friend thought it was assertion failure too. And we found this which we think is asserting that the code must run on main thread, is this true?

https://dxr.mozilla.org/mozilla-central/source/mfbt/Assertions.h#256

Comment 8

[noitidart]

Oops I mean this: http://mxr.mozilla.org/mozilla-release/source/dom/workers/RuntimeService.cpp#1285

MOZ_ASSERT(!NS_IsMainThread());

Comment 9

[Tooru Fujisawa [:arai]]

The assertion thing have no relation to glib.
It's asserting *not* on main thread, because it's using worker's runtime (might be as a part of js-ctypes ffi callback invocation).  The assertion failure happens because you called a JS callback function on main thread, while the function is created on worker thread.  That's the add-on's bug, not firefox's one.

Comment 10

[:Gijs (he/him)]

It sounds like this is essentially invalid. noitidart, you should be able to use a debugger like gdb to break and then call DumpJSStack() to find out which JS is triggering the assertion. It doesn't sound like it's related to the GTK APIs.

Comment 11

[noitidart]

Ah ok. Forgive that please. Thanks still for leaving it open while you had the hunch it didn't belong.

Comment 12

[noitidart]

Big thanks to the rest too for the replies (Arai and Karl!)

Comment 13

[noitidart]

Oh im a bit curious how does firefox do multithreaded stuff as of now? Do they use add_idle?

Comment 14

[Karl Tomlinson (:karlt)]

(In reply to noitidart from comment #13)
> Oh im a bit curious how does firefox do multithreaded stuff as of now? Do
> they use add_idle?

AFAIK most of the thread management is done through nsIThreadManager.
https://dxr.mozilla.org/mozilla-central/search?q=nsIThreadManager&case=true

Firefox only uses GLib apis for platform integration in the GTK port, which code is not usually required on more than one thread.

Comment 15

[noitidart]

(In reply to Karl Tomlinson (ni?:karlt) from comment #14)
> (In reply to noitidart from comment #13)
> > Oh im a bit curious how does firefox do multithreaded stuff as of now? Do
> > they use add_idle?
> 
> AFAIK most of the thread management is done through nsIThreadManager.
> https://dxr.mozilla.org/mozilla-central/search?q=nsIThreadManager&case=true
> 
> Firefox only uses GLib apis for platform integration in the GTK port, which
> code is not usually required on more than one thread.

Ah thanks, Ill work on figuring out how to get this native screenshot thing done off main thread and share with you all. :)

Comment 16

[noitidart]

(In reply to Karl Tomlinson (ni?:karlt) from comment #14)
> (In reply to noitidart from comment #13)
> > Oh im a bit curious how does firefox do multithreaded stuff as of now? Do
> > they use add_idle?
> 
> AFAIK most of the thread management is done through nsIThreadManager.
> https://dxr.mozilla.org/mozilla-central/search?q=nsIThreadManager&case=true
> 
> Firefox only uses GLib apis for platform integration in the GTK port, which
> code is not usually required on more than one thread.

Hey Karl, so I worked on figuring this out with arai. So we found that its that gtk g_add_idle design is not compatible with firefox design. here is gdb:
https://gist.github.com/Noitidart/6d63c967cf59a9231bcb#file-gistfile1-txt-L69

So we set up the javascript callback in the worker: https://github.com/Noitidart/ChromeWorker/blob/patch-2/myWorker.js#L112-L129

And then it crashes when it tries to call the js callback. Its looking for the js function on the main thread. And its not possible for this js function to be on the main thread as in production adodn, it relies on imported jsm. And also for OSX and WinAPI and other stuff, callbacks are on the worker. So from main thread we do myWorker.post('blah'...) so we would have to change it so myWorker is a real worker for OSX and WInAPi but if GTK then we need to make load of worker actually import all the jsm's to the main thread. This would be very hard for average addon devs.

So would it be possible if we re-open this bug to just add a gtk_init_threads on firefox startup, so addon devs can use enter/leave_thread from their workers? so it doesnt require drastic hacking up worker initialization depending on if gtk or not?

Comment 17

[noitidart]

Ahhhh wait wait, so if its deprecated in GDK3 then our solution would have to be to make the g_add_idle design be compatibile with Firefox no?

Comment 18

[Tooru Fujisawa [:arai]]

(In reply to noitidart from comment #16)
> Hey Karl, so I worked on figuring this out with arai. So we found that its
> that gtk g_add_idle design is not compatible with firefox design. here is
> gdb:

To be clear, it's just not compatible with the way: "calling the API from worker via js-ctypes, with JS callback".  Again, it's not a firefox's issue.

Comment 19

[noitidart]

Ah ok so it only affects firefox addon devs trying to use GTK/GDK. Ill put in docs that they shouldn't use GTK/GDK if from Workes and should use X11. Ill start writing the X11 version and hopefully don't run into this issue. :)

Comment 20

[noitidart]

X11 version ready and it seems to not cause any issues!! This is real cool: https://github.com/Noitidart/ChromeWorker/blob/ee53213beec7e1d67b24fde43e0d16ed50938d6d/myWorker.js


However I'm worried for when I have to write up GIOFileMonitor, i actually have it written up and its kind of crashing but i dont know yet if its due to the callback: https://github.com/Noitidart/jscFileWatcher/blob/gio/modules/workers/FSWatcherWorker.js#L164

I have to research that more later, but based on the research above by arai and i, do you think that GIOFileMonitor callbacks will work?

Comment 21

[Karl Tomlinson (:karlt)]

(In reply to noitidart from comment #20)
> I have to research that more later, but based on the research above by arai
> and i, do you think that GIOFileMonitor callbacks will work?

I don't know what thread will run the callbacks.  I would guess the main thread because I expect it needs a Glib event loop context.
I expect it can be made to work.

Comment 22

[noitidart]

(In reply to Karl Tomlinson (ni?:karlt) from comment #21)
> (In reply to noitidart from comment #20)
> > I have to research that more later, but based on the research above by arai
> > and i, do you think that GIOFileMonitor callbacks will work?
> 
> I don't know what thread will run the callbacks.  I would guess the main
> thread because I expect it needs a Glib event loop context.
> I expect it can be made to work.

Thanks karl for reply it would real awesome if we could figure this out. I would love to offer people the option of doing gtk/gdk stuff from workers.