GNOME Bugzilla – Bug 766660
Please clarify the extent to which GInitable, GAsyncInitable must be idempotent
Last modified: 2017-03-30 09:25:21 UTC
The documentation for GInitable says: """ Implementations of this method must be idempotent, i.e. multiple calls to this function with the same argument should return the same results. Only the first call initializes the object, further calls return the result of the first call. This is so that it's safe to implement the singleton pattern in the GObject constructor function. """ However, this is often rather difficult in practice, and most of the GInitable objects shipped in GLib don't even try. For instance, GDBusObjectManagerClient's initable_init() asserts that manager->priv->connection is NULL, which is clearly not idempotent. If the initialization involves several synchronous disk I/O operations, one natural thing to want to do is to delegate them to another thread, like the default implementation of g_async_initable_init_async() does. This has efficiency and code-clarity advantages over performing multiple async GFile transactions, each of which would in practice involve posting requests in the form of idle GSources to threads in a thread pool, and waiting for their responses to be posted back via g_main_context_invoke() or similar. For objects that are not known to be thread-safe, it would normally be sufficient for the initable_init() implementation to take this form (pseudocode): if self->init_error: raise self->init_error else if self->initialized: return success try do synchronous file I/O parse results do more synchronous file I/O etc. catch (any error) self->init_error = that error raise that error return success This is idempotent in the sense that this works, if we assume single-threadedness: my_initable = g_object_new (...) my_initable.init() my_initable.init() However, it is not robust against concurrent initialization, if the object is AsyncInitable. Simplest case (pseudocode): my_initable = g_object_new (...) my_initable.init_async(lambda result: store result) my_initable.init_async(lambda result2: store result2) while (!result or !result2) iterate main context In particular, if the default implementation of g_async_initable_init_async() is invoked multiple times on the same object without waiting for the previous operations to complete, it will happily invoke initable_init() multiple times in different threads. Am I reading too much into this requirement? Is there some other factor that weakens what we require in practice? I find myself trying to fix this with GTasks, mutexes, etc. whenever I write or review an AsyncInitable object, which involves significant complexity for probably little practical benefit (particularly if I know my object is final and does not actually use the singleton pattern in practice); so I would like to clarify exactly what is needed, then make sure there is a pedantically-correct example in GLib.
(In reply to Simon McVittie from comment #0) > The documentation for GInitable says: > > """ > Implementations of this method must be idempotent, i.e. multiple calls to > this function with the same argument should return the same results. Only > the first call initializes the object, further calls return the result of > the first call. This is so that it's safe to implement the singleton pattern > in the GObject constructor function. > """ I agree that this doesn't match how we use GInitables. I don't even know what that last sentence is supposed to mean... I would say that the rule (both as a description of current GLib APIs and as a recommendation for GLib-based APIs in general) is that no public API [besides g_object_new()] should ever return an un-inited GInitable/GAsyncInitable or an initable for which init()/init_async() failed. Given that, it doesn't even make sense to talk about whether init() is idempotent or not, because there is no reason you would ever want to call it on an object that it has already been called on. (It's possible some people might do things internal to their own code that involve maybe-inited-maybe-not objects, but in that case, they are free to solve the problem with whatever internal-to-their-own-code solution they want to.)
I agree, lets drop this from the docs.
Created attachment 329865 [details] [review] ginitable: Relax idempotency requirements on init() and init_async() The previously documented requirements for implementing init() and init_async() as completely idempotent were really quite hard to achieve, and brought a lot of pain for very little gain. Many implementations of GInitable and GAsyncInitable did not actually follow the requirements, or did not correctly handle concurrent init_async() calls. Relax those requirements so that classes can decide whether their init() or init_async() implementations need to be idempotent.
Review of attachment 329865 [details] [review]: Having implemented and reviewed several initables which failed this test, I like this simplification. It would probably be worthwhile to mention the singleton pattern as a common reason why one might wish to make these functions idempotent, perhaps something like: One reason why a class might need to support idempotent initialization is if it is designed to be used via the singleton pattern, with a #GObjectClass.constructor that sometimes returns an existing instance. In this pattern, a caller would expect to be able to call g_initable_init() or g_async_initable_init_async() on the result of g_object_new(), whether it is in fact a new instance or not.
(In reply to Dan Winship from comment #1) > (In reply to Simon McVittie from comment #0) > > The documentation for GInitable says: > > > > """ > > [...] This is so that it's safe to implement the singleton pattern > > in the GObject constructor function. > > """ > > I agree that this doesn't match how we use GInitables. I don't even know > what that last sentence is supposed to mean... I assume it's this pseudocode, as seen in tests/gobject/singleton.c: static MyObject *the_object; static void my_object_constructor (MyObjectClass *cls, ...) { if (the_object == NULL) { the_object = (chain up to parent constructor); take a weak reference that will clear the_object on dispose; } else { g_object_ref (the_object); } return the_object; } which can result in g_object_new() returning a reference to an existing object.
Created attachment 330591 [details] [review] ginitable: Relax idempotency requirements on init() and init_async() The previously documented requirements for implementing init() and init_async() as completely idempotent were really quite hard to achieve, and brought a lot of pain for very little gain. Many implementations of GInitable and GAsyncInitable did not actually follow the requirements, or did not correctly handle concurrent init_async() calls. Relax those requirements so that classes can decide whether their init() or init_async() implementations need to be idempotent.
Anyone?
Review of attachment 330591 [details] [review]: Like, really.... I mean, I'd just erase the offending line and not bother talking about it. The one thing that I might add is a note: "you don't want to call this function; use g_initable_new() instead.".
How about I make one of these chunks a cross-reference to the other, and trim it down to a sentence for class implementers about how idempotence is not a requirement any more.
Pushed with some trimming, cross-referencing and big arrows pointing to g_initable_new() instead. Attachment 330591 [details] pushed as deab643 - ginitable: Relax idempotency requirements on init() and init_async()