GNOME Bugzilla – Bug 563627
g_get_prgname() threadsafety
Last modified: 2010-02-02 17:40:22 UTC
The docs for g_set_prgname() say: "Note that for thread-safety reasons this function can only be called once." However, the code doesn't enforce that. So a string obtained using g_get_prgname() can later be freed if g_set_prgname() is called again. I suggest g_set_prgname() do a g_return_if_fail (prgname == NULL);
We should use something like g_once() actually, to make sure we don't introduce a race condition as described by your proposal.
Created attachment 146099 [details] [review] proposed patch v1
(In reply to comment #2) > Created an attachment (id=146099) [details] [review] > proposed patch v1 Looks good Sven, please apply.
Committed. http://git.gnome.org/cgit/glib/commit/?id=3dab24828edd8ebfd6d6c8bb20c181fc0d5650a1
make check fails in glib/tests/option-context now.
(In reply to comment #5) > make check fails in glib/tests/option-context now. Thanks Benjamin, seems g_option_context_parse() calls g_set_prgname() unconditionally. This needs to be fixed as well, i.e.: - g_option_context_parse shouldn't override a prgname if has been previously set by an applicaiton (i.e. it should only set the prgname if it's unset). - programs might validly change the prgname that has been set by g_option_context_parse, which means that g_set_prgname() needs to be callable more than once. So we'll have to revert the patch. Please note that because g_option_context_parse() calls g_set_prgname(), we have also broken applications that currently do: main() { g_option_context_parse (...); g_set_prgname (...);
The check inhibiting use of g_set_prgname() just turned up in Glib 2.22.3; but Tim's suggestion that > - programs might validly change the prgname that has been set by > g_option_context_parse, which means that g_set_prgname() needs to be callable > more than once. seems not to work anymore. Does that mean that the only correct behaviour is to call g_set_prgname() before [say] gtk_init()? AfC
We likely need to back out the current patch because it's causing warnings in too many programs and breaks the testsuite. And apparently there hasn't been any progress to resolve the issue otherwise.
(In reply to comment #8) > We likely need to back out the current patch because it's causing warnings in > too many programs and breaks the testsuite. It should simply be backed out to fix those cases. > And apparently there hasn't been any progress to resolve the issue otherwise. There's really no alternative to unbreak our API again.
I don't know if you have taken this into account, but note that any gtk-program that has --name <name> in its argument list will get the warning about g_set_prgname() called multiple times when gtk_init is called. And this is all inside gtk_init, I don't know what an app should do to get around it. Apparently Gdk has an option callback for --name that does g_set_progname.
Fwiw, I agree. I was quite surprised when I had to disable one of the option-context tests to fix make check, due to this change.
Reverted now.
(In reply to comment #12) > Reverted now. Thanks a lot, WONTFIXing this, since due to technical reasons we need g_get_prgname() to be callable multiple times.