Skip to content

gmain: drop owner check assertion in g_main_context_release()

Thomas Haller requested to merge th/fix-main-context-release-owner-check into main

As commit 44616eba ('gmain: More explicitly document g_main_context_release() prereqs') correctly notes, you need to have the context acquired before releasing it (just like a ref must match an unref).

However, commit 3926af72 ('gmain: Add precondition assertions to g_main_context_release()') then goes one step further, and requires that the calling thread is also the owner (the thread, that acquired the context).

With acquire/release and g_main_context_is_owner() we track the thread that acquired the context. That is mainly useful for asserting correctness to not accessing the context from an unexpected thread. Note that g_main_context_acquire() returns FALSE and does nothing when the context is already acquired from another thread. Methods like g_main_context_{prepare,query,dispatch}() require that the calling thread is the owner (although, they don't assert for that, which they maybe should).

For g_main_context_release() however, it's an unnecessarily strict requirement that the calling thread owns the context. The release is like an unref (which must match the number of successful acquire calls), it should generally just work to release the resource, from any thread.

With the assertion, it means you cannot pass an acquired context to another thread for release. Obviously, if you pass on an acquired context to another thread, the only next thing you can do is g_main_context_release() (no acquire,prepare,query,dispatch). But it's still useful to be able to release it, and to be able to keep it acquired for a prolonged time.

libnm needs that, as it integrates a GMainContext into another GMainContext. For that, it needs to acquire the inner context and keep it acquired for as long as the context is integrated. Otherwise, when a source gets attached to the inner context, the inner context is not woken up (see g_source_attach_unlocked()). In commit e26a8a59 ('Add G_MAIN_CONTEXT_FLAGS_OWNERLESS_POLLING'), a flag was introduced to solve that same problem, without keeping the inner context acquired all the time. Note that G_MAIN_CONTEXT_FLAGS_OWNERLESS_POLLING is a flag of the GMainContext, so it only works if the user who integrates the inner context also controls how the context was created. For libnm, having the inner context acquired at all times is no problem, because it's understood that the user gives up agency on the inner context while it's integrated. The only thing to consider is that the outer context might be iterated on another thread. When calling prepare,query,dispatch on the inner context, the code will notice it, release the inner context and re-acquire it on the new thread ([1]). This works just fine, but it requires that g_main_context_release() works from any thread.

Still keep the assertion for the owner_count.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/9f01cff04f4a0aac5e4438e6bdb0e5c68594a706/src/libnm-glib-aux/nm-shared-utils.c#L4944

Fixes: 3926af72 ('gmain: Add precondition assertions to g_main_context_release()') Fixes: c67dd9d3 ('gmain: Add a missing return on error path in g_main_context_release()')

#3054 (closed)

Edited by Thomas Haller

Merge request reports