GNOME Bugzilla – Bug 662208
failure to initialize a GInitable should be considered programming error
Last modified: 2011-11-23 13:03:37 UTC
+++ This bug was initially created as a clone of Bug #661689 +++ <davidz> alexl: hey - re https://bugzilla.gnome.org/show_bug.cgi?id=661689 I'm in favor of just changing the GInitable docs (e.g. replace "should return G_IO_ERROR_NOT_INITIALIZED" with "should g_warning(), g_critical") ... I just don't see how it can ever be valid to use a GInitable instance until it has been initialized (but maybe I'm not looking hard enough)... thoughts? <alexl> davidz: Can't think of any useful scenario either. <alexl> davidz: go for it <davidz> alexl: sure, I'll follow up in the bug davidz then wrote: > Thus, I think we should change the GInitable docs to suggest that > use-before-initialization is a programming error in the general case ... and > that programmers should e.g. g_return_if_fail(), g_critical() instead of > returning G_IO_ERROR_NOT_INITIALIZED. Thoughts? Dan Winship replied: > agreed, and likewise for use-after-initialization-has-failed; the only thing > you can do if g_initable_init() returns FALSE is destroy the object. > > The GInitable docs should probably also xref GAsyncInitable a bit more. > (eg, if an object is both GInitable and GAsyncInitable, then you can > call either g_initable_init() or g_async_initable_init_async() to > initialize it). > > Also, presumably you are allowed to g_object_ref/unref() an initable before > initializing it, which the docs currently technically forbid. I'll write some patches for master for this, but I don't think it'd be appropriate to make this change in a stable branch.
One argument against this is that (particularly if applied to properties and other generic GObject features) it makes both GInitable and GAsyncInitable into "anti-interfaces" - anything that interacts with GObjects generically is considered broken, unless it knows about them. In particular, making an object implement an extra interface is normally a "safe" change that doesn't affect its ABI - everything that was valid before is still valid - but this isn't the case for the initable interfaces (although if you implement one, implementing the other is safe). GInitiallyUnowned is a bit like this too - replacing a direct GObject subclass with a GInitiallyUnowned subclass can turn correct code into incorrect code. (I agree that raising a GError makes very little sense, though.)
(In reply to comment #1) > In particular, making an object implement an extra interface is normally a > "safe" change that doesn't affect its ABI - everything that was valid before is > still valid - but this isn't the case for the initable interfaces (although if > you implement one, implementing the other is safe). That's always been true though. If an object is not initable, you're obviously not going to have code checking if its methods return G_IO_ERROR_NOT_INITIALIZED, so if someone makes it initable later, you'll lose under either the current rules or the propsed new rules.
(In reply to comment #1) > One argument against this is that (particularly if applied to properties and > other generic GObject features) it makes both GInitable and GAsyncInitable into > "anti-interfaces" - anything that interacts with GObjects generically is > considered broken, unless it knows about them. > > In particular, making an object implement an extra interface is normally a > "safe" change that doesn't affect its ABI - everything that was valid before is > still valid - but this isn't the case for the initable interfaces (although if > you implement one, implementing the other is safe). No one says you MUST NOT implement your type implementing GInitable in a way so using an instance of it before initialization is not considered a programming error.. IOW, it is valid to implement a type FooBar implementing GInitable and have one of its methods fail with G_IO_ERROR_NOT_INITIALIZED (rather than g_critical() etc) if it hasn't been successfully initialized. I think all we're saying is that we RECOMMEND that people implement their types so it is considered a programming error if an instance of the type is used before it has been successfully initialized. And if we say this, we should probably stress that an application MAY NOT consider it a programming error if an instance is used before being successfully initialized. I think the easiest thing is probably to say nothing at all in the GInitable docs... instead we'd mention in each GInitable type (such as GDBusConnection) that it is considered a programming error if an instance is used before being successfully initialized. (and sorry for the shouting - I'm only shouting in the RFC 2119 sense :-) )
(In reply to comment #3) > And if we say this, we should > probably stress that an application MAY NOT consider it a programming error if > an instance is used before being successfully initialized. ... > (and sorry for the shouting - I'm only shouting in the RFC 2119 sense :-) ) The reason that RFC 2119 doesn't give "MAY NOT" a meaning is that it's ambiguous :-) but I think what you mean is: an object implementation MAY consider method calls on an uninitialized instance to be either valid or invalid. > I think the easiest thing is probably to say nothing at all in the GInitable > docs... instead we'd mention in each GInitable type (such as GDBusConnection) > that it is considered a programming error if an instance is used before being > successfully initialized. Thanks, I think this approach is better than redefining what GInitable means. In particular, GDBusConnection would mostly crash anyway - so I think it's OK to critical all over the place there - but I don't think it'd be a good idea to change all GSocket's graceful GErrors into criticals. Patch on the way...
(In reply to comment #3) > No one says you MUST NOT implement your type implementing GInitable in a way so > using an instance of it before initialization is not considered a programming > error.. I do! I do! :) Language bindings will call g_initable_init() for you automatically at construction time, so if there's some legitimate use case where a caller wouldn't want to initialize your type right away, then you shouldn't be using GInitable. You should just have your own initialize/prepare/connect/whatever method, and document how it should be used. The original idea behind GInitable was "g_object_new() that can return a GError". No one should ever see an initable object that has been constructed but not yet inited, unless the object hands out a reference to itself from within constructed() or init().
(In reply to comment #5) > No one should ever see an initable object that has been constructed > but not yet inited, unless the object hands out a reference to itself from > within constructed() or init(). ... or unless you call g_object_new() on it, which is how language bindings always used to make GObjects (if I understand correctly). (The crash dumps that prompted this extended yak-shaving excursion indicated that GDBusConnection might be leaking one of its internal uninitialized refs out to user code... or the devices where the crashes happened might have had bad RAM, or there might have been heap corruption. We just don't know.)
(In reply to comment #5) > (In reply to comment #3) > > No one says you MUST NOT implement your type implementing GInitable in a way so > > using an instance of it before initialization is not considered a programming > > error.. > > I do! I do! :) > > Language bindings will call g_initable_init() for you automatically at > construction time, so if there's some legitimate use case where a caller > wouldn't want to initialize your type right away, then you shouldn't be using > GInitable. You should just have your own initialize/prepare/connect/whatever > method, and document how it should be used. > > > The original idea behind GInitable was "g_object_new() that can return a > GError". No one should ever see an initable object that has been constructed > but not yet inited, unless the object hands out a reference to itself from > within constructed() or init(). I agree with all that. But consider someone retrofitting a type FooBar to use GInitable - maybe it's nice to have old C-based g_object_new()-based code working while transitioning (maybe it's a huge distributed code-base)... in particular, the exsiting initialize/prepare/connect/whatever method could simply be calling g_initable_init().
Created attachment 199583 [details] [review] GInitable, GAsyncInitable: not initializing gives undefined behaviour This is the ISO C sense of undefined behaviour, in which works-by-coincidence, critical warning, abort, demons-fly-out-of-your-nose are all valid implementations. --- This is the strictest version (i.e. what Dan said), but also the version that most directly contradicts the previous documentation. I'm open to suggestions.
Can it start NetHack? (http://www.feross.org/gcc-ownage/)
(In reply to comment #9) > Can it start NetHack? (http://www.feross.org/gcc-ownage/) I'm now very tempted to add that somewhere. system() is portable, right? :-)
Review of attachment 199583 [details] [review]: Looks good to me. Thanks for doing this.
I'll leave this for a couple of days to give others a chance to veto/comment/approve, since it's pure documentation and doesn't really block anything (particularly now that GDBusConnection documents that you really do have to initialize first).
(In reply to comment #12) > I'll leave this for a couple of days to give others a chance to > veto/comment/approve Nobody did, so I merged it to master for 2.31.3. Commit 0104c62.