g_array_free() is not guaranteed to be thread-safe — the only methods of GArray and GPtrArray which are guaranteed to be thread-safe are the ref() and unref() methods. If you want to access the data from a single GArray or GPtrArray instance from multiple threads, you must provide your own locking.

This is inline with the rest of GLib, which is not thread-safe unless explicitly advertised as such.

It's clear that the g_array_foo() functions are not generally thread safe with respect to each other (or themselves, for that matter), it's far less clear that they're not thread safe with respect to _ref()/_unref() (as long as each thread guarantees the existence of a reference while it's using the array, obviously).  This seems to vastly reduce the usefulness of _ref()/_unref() being thread safe to begin with -- it doesn't "feel" right.

Off the top of my head, I can't think of any other case where ref()ing/unref()ing an object would clash with another method on another thread (once again, as long as the other thread has a reference on the object -- i.e., you don't pull the last reference from under it.)

Except that g_array_free() interacts with the reference counting, so unless it’s documented as being thread-safe, it should not be considered to be thread-safe wrt g_array_[un]ref().

Would it help if this was made more explicit in the documentation?

Do you have a particular situation in mind where it makes sense to control the ref count of a GArray from one thread, while interacting with the data it contains from another?

I shamelessly admit that this bug is purely hypothetical -- I'm not actually trying to do anything (sorry :)  While I don't have a specific example to give, that's partly the point: ref count control is so pervasive, it can happen as part of so many things, that it's hard to predict what else might be going on at the same time.  It seems to me that this is what makes having it be thread safe (wrt whatever) useful -- otherwise you'd need a lock just to drop a reference to an object in a destructor somewhere.  On the other hand, I'm having a harder time thinking of an example where you'd need ref counting to be thread safe *only* wrt itself.

If the ref()/unref() functions are not generally thread safe wrt other functions (in which case there's indeed nothing special about g_array_free()), then making *that* explicit is definitely a good idea (to be absolutely pedantic, the docs don't even say that ref()/unref() are thread safe wrt each other :)

OTOH, if g_array_free() is the only odd one out, it seems easier to just fix it.

(In reply to Ell from comment #4)
> otherwise you'd need a lock just
> to drop a reference to an object in a destructor somewhere.

Nope, since the reference count functions only become thread-unsafe wrt other functions if you’re dropping the *last* ref to an object. If you’re doing that and calling other methods on the object (from any thread), then you’ve got a refcounting bug — somewhere in the code is not taking a ref when it should.

> If the ref()/unref() functions are not generally thread safe wrt other
> functions (in which case there's indeed nothing special about
> g_array_free()), then making *that* explicit is definitely a good idea (to
> be absolutely pedantic, the docs don't even say that ref()/unref() are
> thread safe wrt each other :)

I’ll push a documentation clarification.

commit 2586eb992193ee9226a7a2b92c1c644347e9a842 (HEAD -> master, origin/master, origin/HEAD)
Author: Philip Withnall <withnall@endlessm.com>
Date:   Tue Aug 22 15:58:18 2017 +0100

    docs: Clarify lack of threading guarantees in GArray
    
    Signed-off-by: Philip Withnall <withnall@endlessm.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786555

Ah, I should have been more clear.  The situation I'm talking about is this:

  - Array A has a ref count of *2*.
  - Thread X calls g_array_free() on A.
  - At the same time, thread Y calls g_array_unref() on A.

There's a race between the two functions, since g_array_free() drops its reference *before* stealing/freeing the data, rather than after.  If thread Y is scheduled in the middle of this, bad things happen.

Yes, and the bug is there because either g_array_free() or g_array_unref() is dropping the last ref to the array. If you really want to do this safely without using your own locks, use the following instead of plain g_array_free():

g_array_ref (A);
data = g_array_free (A, FALSE);
g_array_unref (A);

which ootomh should be safe. Obviously that’s a bit of a ridiculous pattern to use by itself, but it makes a bit more sense in the wider context of a function.