Assert vs. Critical

Platform
1

Hi,

In the last weeks, I hit twice a invalid_closure_notify assert in glib. That makes the app immediately crash.

“git blame” points to this old ticket Bug 730296 – gsignal: Fix a potential NULL pointer dereference , which seems to acknowledge that the handler == NULL case may actually occur. And I’m a living proof it can :slightly_smiling_face:

In this case, wouldn’t it be better to raise a critical instead?
Is the error bad enough to justify fully crashing the application?

Something like this:

  if (handler)
    {
      g_assert (handler->closure == closure);

      g_hash_table_remove (g_handlers, handler);
      handler->sequential_number = 0;
      handler->block_count = 1;
      handler_unref_R (signal_id, instance, handler);
    }
  else
    g_critical (G_STRLOC ": invalid closure notify with NULL handler for instance '%p'",
                instance);
2

Basically the rule is:

  • If there’s a bug in GLib, it should use g_assert()
  • If there’s a bug in the application, it should use g_critical()

Either way, it’s a bug and shouldn’t happen, and we should try to figure out why. What app is crashing? Can you get a stack trace?

Looking at the bug report you linked to, I agree that g_critical() might potentially be more appropriate here, but we’d need to understand what’s happening before changing anything. Also, we’d need to find a way to make the critical message understandable to application developers rather than referencing confusing GLib implementation details.

3

Thanks!

A private pygobject+gtk3 app.

Saw it once on Windows11 (msys2-ucrt64), and once on Linux (X11).
Both with latest libraries (gnome-48 versions of glib and gtk3).

I will try to get one, but the bug is very rare, I haven’t a reproducer for now.

It started to occur quite recently, so may be related to some recent change in glib or gtk3 or pygobject…

Indeed.

Sure. The current assert message is quite abstruse…