Should I call g_object_weak_unref() during dispose?

[madmurphy]

At the Object memory management page of the manual (§ Weak References) I read (emphasis mine):

Weak references are used to monitor object finalization: g_object_weak_ref adds a monitoring callback which does not hold a reference to the object but which is invoked when the object runs its dispose method. As such, each weak ref can be invoked more than once upon object finalization (since dispose can run more than once during object finalization).

g_object_weak_unref can be used to remove a monitoring callback from the object.

That makes me think that I must first write:

g_object_weak_ref(
	my_object,
	on_my_object_dispose,
	my_data
);

And then the on_my_object_dispose() callback should be written so that it unrefs the weak reference:

static void on_my_object_dispose (
	const gpointer my_data,
	GObject * const my_object
) {

	g_object_weak_unref(
		my_object,
		on_my_object_dispose,
		my_data
	);

	// DO SOMETHING

}

But if I do so, I get

GLib-GObject-CRITICAL **: 22:13:19.781: g_object_weak_unref: couldn't find weak ref 0x7f2730e88770(0x5591d8bad760)

So it seems that it was alread unreferenced. If I don’t call g_object_weak_unref() everything works fine.

What is the correct thing to do?

[ebassi (Emmanuele Bassi)]

It seems you did not read the documentation for GWeakNotify: weak references on the object are automatically dropped when the instance is disposed, so by the time your weak reference handler is called, the reference is already gone.

You’re also using the weak reference the wrong way; I mean: you’re passing a const pointer to a function that doesn’t take one. Aren’t you getting a compiler warning from that? If you aren’t, you should turn on more warnings, because you’re likely doing something else wrong.

The function signature for the weak reference callback is:

void
(* GWeakNotify) (
  gpointer data,
  GObject* where_the_object_was
)

The where_the_object_was is a pointer useful for using the address of the object, like removing an object from a cache; the instance itself is already disposed.

[madmurphy]

@ebassi

It seems you did not read the documentation for GWeakNotify: weak references on the object are automatically dropped when the instance is disposed, so by the time your weak reference handler is called, the reference is already gone.

Hi Emmanuele. You are right. I had read it back then when I wrote my code, but I did not read it again now that I was reviewing my code (I stopped at the passage quoted and posted it here). So I was correct back then.

But then maybe that passage could be written more clearly? These words really seems to suggest that I have to drop the reference manually myself by calling g_object_weak_unref(): “a monitoring callback which … is invoked when the object runs its dispose method. As such, each weak ref can be invoked more than once upon object finalization (since dispose can run more than once during object finalization). g_object_weak_unref can be used to remove a monitoring callback from the object.”

Or maybe it’s just me reading it that way…

Aside point:

@ebassi

you’re passing a const pointer to a function that doesn’t take one. Aren’t you getting a compiler warning from that?

I always use -pedantic (recently I even submitted a commit to GTK because of that). The reason I am not getting a warning is that it is not a gconstpointer, the constantness applies only to the pointer and not what is pointed. If it is true that

1. gpointer foobar is equal to void * foobar
2. const gpointer foobar is equal to void * const foobar
3. gconstpointer foobar is equal to const void * foobar
4. const gconstpointer foobar is equal to const void * const foobar

Then only #3 and #4 indicate that the memory pointed to is constant (and I would get a warning). Instead #1 and #2 indicate that the pointer will always point to the same location, but the location is not constant. The constantness of the pointer is totally opaque during a function call, it is not transmitted (unless you pass the address – i.e. &foobar). Therefore you can increase the warnings as much as you like but you will never get a warning for the case in point, because it is correct. The const keyword simply indicates that I am not going to change the location that is pointed – because indeed I never change the location pointed.

[ebassi (Emmanuele Bassi)]

@madmurphy

But then maybe that passage could be written more clearly?

What do you mean with “written more clearly”? It’s pretty clear:

Since the object is already being disposed when the GWeakNotify is called, there’s not much you could do with the object, apart from e.g. using its address as hash-index or the like.

In particular, this means it’s invalid to call g_object_ref(), g_weak_ref_init(), g_weak_ref_set(), g_object_add_toggle_ref(), g_object_weak_ref(), g_object_add_weak_pointer() or any function which calls them on the object from this callback.

It says that you cannot use the GObject instance for anything because it has already been disposed, and at most you can use its address. Manipulating the references of an object—both weak and strong—is not allowed.

@madmurphy

I always use -pedantic (recently I even submitted a commit to GTK because of that).

Never, ever use -pedantic with GLib-based code: it cannot work. The toolchain requirement on function pointer conversion prevent the use of -pedantic.

[madmurphy]

@ebassi

What do you mean with “written more clearly”? It’s pretty clear:

I mean the passage that I quoted, not the one from GWeakNotify – you can find the same passage here in the new documentation (under § Weak References).

@ebassi

Never, ever use -pedantic with GLib-based code: it cannot work. The toolchain requirement on function pointer conversion prevent the use of -pedantic.

Ok, I did not know that. However, for the sake of history, I never encountered any problems so far (so I was probably lucky). And when I encountered warnings, like in the functions generated by glib-genmarshal, using -pedantic allowed me to correct them and make them work following the standard (if I recall correctly the warnings came from casting a data pointer to a function pointer or vice versa, but there is a way to do that without warnings in -pedantic mode).

In any case, I never add -pedantic to my build system, I only use it when I compile the code manually while I am still heavily writing. After I do that, I drop the -pedantic directive and stick only to -Wall for the build system. So no problems should arise. In any case I will pay particular attention to it now.

[madmurphy]

@ebassi

I remember now! If you are interested, this is how I corrected the code generated by glib-genmarshal (it was a rather articulated callback that I needed). This version does not generate warnings in -pedantic mode (nor gave me any problem so far – don’t ask me what I changed back then, because I don’t remember – but it was definitely something concerning the syntax of pointer casting):

/**

    gnui_cclosure_marshal_VOID__STRING_ENUM_ENUM_POINTER:

    Generated by `glib-genmarshal` with `VOID:STRING,ENUM,ENUM,POINTER`

**/
static void gnui_cclosure_marshal_VOID__STRING_ENUM_ENUM_POINTER (
	GClosure * const closure,
	GValue * const return_value G_GNUC_UNUSED,
	const guint n_param_values,
	const GValue * const param_values,
	const gpointer invocation_hint G_GNUC_UNUSED,
	const gpointer marsh_data
) {

	#ifdef G_ENABLE_DEBUG
	#define g_marshal_value_peek_enum(v) g_value_get_enum (v)
	#define g_marshal_value_peek_string(v) (char*) g_value_get_string (v)
	#define g_marshal_value_peek_pointer(v) g_value_get_pointer (v)
	#else
	/*
	    WARNING: This code accesses GValues directly, which is UNSUPPORTED API.
	             Do not access GValues directly in your code. Instead, use the
	             g_value_get_*() functions
	*/
	#define g_marshal_value_peek_enum(v) (v)->data[0].v_long
	#define g_marshal_value_peek_string(v) (v)->data[0].v_pointer
	#define g_marshal_value_peek_pointer(v) (v)->data[0].v_pointer
	#endif

	g_return_if_fail(n_param_values == 5);

	typedef void (* GMarshalFunc_VOID__STRING_ENUM_ENUM_POINTER) (
		gpointer data1,
		gpointer arg1,
		gint arg2,
		gint arg3,
		gpointer arg4,
		gpointer data2
	);

	gpointer data1, data2;

	if (G_CCLOSURE_SWAP_DATA(closure)) {

		data1 = closure->data;
		data2 = g_value_peek_pointer(param_values + 0);

	} else {

		data1 = g_value_peek_pointer(param_values + 0);
		data2 = closure->data;

	}

	(
		*(
			(GMarshalFunc_VOID__STRING_ENUM_ENUM_POINTER *) (
				marsh_data ? &marsh_data : &((GCClosure *) closure)->callback
			)
		)
	)(
		data1,
		g_marshal_value_peek_string(param_values + 1),
		g_marshal_value_peek_enum(param_values + 2),
		g_marshal_value_peek_enum(param_values + 3),
		g_marshal_value_peek_pointer(param_values + 4),
		data2
	);

	#undef g_marshal_value_peek_pointer
	#undef g_marshal_value_peek_string
	#undef g_marshal_value_peek_enum

}