GNOME Bugzilla – Bug 768230
No safe way to define custom "event" GSource without providing some kind of fd
Last modified: 2018-05-24 18:59:10 UTC
Currently it seems impossible to define a new custom GSource without having it provide a fd and having it thread-safe. Consider a GSource that at some random point in time becomes ready to be dispatched and we know that from some thread. We now could look at the context stored in the GSource and call g_main_context_wakeup() on it, the main context would then go through prepare(), dispatch(), etc again and all would be good. However there is nothing that allows to get the GMainContext in a threadsafe way. It might already be on the way to destruction in g_main_context_unref() when we get it from the GSource, but not set to NULL in the GSource yet. Is there any plan, any ideas to provide some kind of thread-safe g_source_wakeup() or similar that could be used for this kind of stuff? Handling fds as an alternative would work of course, but is rather non-trivial in a platform independent way (see e.g. gwakeup.c).
Couldn't you just pass around a reference to the main context instead and have callers do g_main_context_wakeup()?
> Is there any plan, any ideas to provide some kind of thread-safe > g_source_wakeup() or similar that could be used for this kind of stuff? There's no plan, because this problem hadn't been noticed before... I don't think "thread-safe g_source_wakeup()" is the right approach because then we'd just run into some other problem later with needing the GMainContext for some other call. We need an API that atomically fetches and refs the source's GMainContext, or else returns NULL if the context is already being destroyed. "g_source_ref_context()"? I feel like there are other functions in glib that have these semantics but I can't think of what they are...
(In reply to Colin Walters from comment #1) > Couldn't you just pass around a reference to the main context instead and > have callers do g_main_context_wakeup()? The problem is that as an implementor of the GSource you don't have a way to get the context in a safe way. Some API like Dan suggests is missing, or a callback in GSourceFuns that notifies when it is attached/detached to/from a context. (In reply to Dan Winship from comment #2) > > Is there any plan, any ideas to provide some kind of thread-safe > > g_source_wakeup() or similar that could be used for this kind of stuff? > > There's no plan, because this problem hadn't been noticed before... > > I don't think "thread-safe g_source_wakeup()" is the right approach because > then we'd just run into some other problem later with needing the > GMainContext for some other call. > > We need an API that atomically fetches and refs the source's GMainContext, > or else returns NULL if the context is already being destroyed. > "g_source_ref_context()"? I feel like there are other functions in glib that > have these semantics but I can't think of what they are... That's the other option yes, would also work for me. I could implement that if you think that's the way to go. It will require some minor shuffling around of code in gmain.c but should be fairly trivial to implement otherwise.
Yeah, I think something like g_source_ref_context() is the right way to go. With the docs in g_source_get_context() updated to clarify more when it is and isn't safe to use. In terms of naming, I guess the other API I was thinking of is g_weak_ref_get(), but I don't think there's a good way to give this API a name similar to that one. It would be nice to have a name that more clearly indicated atomicity / thread-safety / weak-reference-strengthening or something though (without making the name too ugly).
g_source_get_and_ref_context()? I have no good ideas for the name. But if you decide that this is the way to go forward, I can make a patch and then we can decide on a name later and do search & replace ;)
Prior art would point to g_source_dup_context(), similarly to the GValue API.
And g_value_dup_*() and others. It doesn't seem right though, but I'll go with that one then for now ;) Thanks for the suggestions, expect a patch soon.
Created attachment 335778 [details] [review] GSource – Add g_source_dup_context() to get the main context in a thread-safe way g_source_get_context() will give invalid memory and no way to protect against that when called on a source where the context is currently being destroyed in another thread. To get around this, we ensure that the context is still in the list of valid main contexts and also still has a reference count higher than 0, and then return a new reference. This allows this function to be used in a thread-safe way inside GSource implementations to wake up the main context whenever the source wants to be polled again, without having to provide its own file descriptor for that.
Ping? :)
The correct way to do this is g_source_set_ready_time(source, 0);. This is thread-safe.
Good, then let's close this. Seems like this needs some more advertisement somehow if Dan and Colin also were not thinking of that one, and I also didn't find it in the docs when searching for this ;)
That doesn't do anything about the race condition. It just moves the "We now could look at the context stored in the GSource and call g_main_context_wakeup() on it" from user code to glib code. It's still trying to act on an object that the source doesn't have a ref on, which might also be being acted on from another thread. Although, we could make it work without adding new public API if we wanted... (Essentially, just add "g_source_dup_context()" as a private function and use it from g_source_set_ready_time().)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1179.