Created attachment 235435 [details] [review]
disable support for unloading of dynamic types

Experimentally disable the ability to unload dynamic types by refusing
to drop the last reference on types (effectively turning the type
unloading into dead code).

The plan is to leave things like this for a stable cycle and only
proceed with removing the code if we are sure that there are no
unforeseen problems.

Created attachment 235438 [details] [review]
tests: comment out asserts in dynamic type tests

We have some testcases that assert that type modules are unloaded after
the last reference on them is dropped.  Comment out those asserts now
that we turned the last unref into a no-op.

Review of attachment 235435 [details] [review]:

ok, lets get this in then.

Attachment 235435 [details] pushed as 72df626 - disable support for unloading of dynamic types
Attachment 235438 [details] pushed as d7c8eda - tests: comment out asserts in dynamic type tests

Bug stays open until we're really done.

We have a reports of this issue being expressed at the LSB

https://lsbbugs.linuxfoundation.org/show_bug.cgi?id=3844

The documentation (for GType/GTypePlugin, etc.) implies rather strongly that a class will be unused when the last instance is freed:

-----QUOTE-----
Object instance destruction through g_type_free_instance is very simple: the instance structure is returned to the instance pool if there is one and if this was the last living instance of the object, the class is destroyed.

…and...

At some point the type's implementation isn't required anymore, e.g. after g_type_class_unref() or g_type_free_instance() (called when the reference count of an instance drops to zero).

This causes the type system to throw away the information retrieved from g_type_plugin_complete_type_info() and then it calls g_type_plugin_unuse() on new_type_plugin .
-----QUOTE-----

I'd like to bump this issue in hopes that this functionality can be restored, perhaps as an experimental or per-class feature.

(In reply to Scott Hutton from comment #6)

> I'd like to bump this issue in hopes that this functionality can be
> restored, perhaps as an experimental or per-class feature.

We don't want to restore this since we find that it is causing more problems than adding useful functionality.

The useful functionality is that (presuming it works) you can reload a shared library without restarting the process, a big win for highly-available server software (probably less so for GUIs and applications that are frequently restarted).

Regardless, the documentation is inaccurate -- it should indicate that dynamically-loaded classes will not be finalized, and that GTypeModules effectively never automatically unuse modules that aren't in use.

Please reconsider allowing this to be overridden on a per-class basis, caveat user.  Then you wouldn't have a dead code path at the end of type_data_unref_U().

(In reply to Scott Hutton from comment #8)
> The useful functionality is that (presuming it works) you can reload a
> shared library without restarting the process, a big win for
> highly-available server software (probably less so for GUIs and applications
> that are frequently restarted).

The issue with that, and what led to the disabling of this feature, is that we cannot really unload the types from the type system, especially for statically registered types. Existing code will leave data in static storage, or will use compiler annotations that will prevent completely removing types from memory, and the next time you load the same library and try to register the same type, you'll get into undefined behaviour. Dynamically registered types can (and will) cause the registration of statically registered types, even inside the same library, which means that we cannot guarantee that unloading a dynamic module will return the type system to its previous state.

For better or worse, we cannot retroactively fix existing code, so the only way to make this work would be to break API compatibility.

> Regardless, the documentation is inaccurate -- it should indicate that
> dynamically-loaded classes will not be finalized, and that GTypeModules
> effectively never automatically unuse modules that aren't in use.

That was kind of part of this bug, as Allison wrote in the commit log in comment #1. I guess, after 3 years, we can say that the experiment succeeded, and we should drop the code.

> Please reconsider allowing this to be overridden on a per-class basis,
> caveat user.

We could have a "I swear my type does not have side effects that will cause the registration of static types as the result of type instantiation" flag, but my experience with that kind of flag is that it gets forgotten when code changes, and then stuff starts to break.

(In reply to Emmanuele Bassi (:ebassi) from comment #9)
> We could have a "I swear my type does not have side effects that will cause
> the registration of static types as the result of type instantiation" flag,
> but my experience with that kind of flag is that it gets forgotten when code
> changes, and then stuff starts to break.

As long as it's an opt-in behavior, it doesn't seem much worse than what's already there with non-plugin types.  There's always the risk of unfreed memory and dangling pointers, regardless of the scenario -- all that really changes is where those dangling pointers point: to a recycled area or a formerly-mapped page.

Even the 'big guys' can't get unloading right, see e.g:

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/KC2ENZFD57SMLJ3ZFX7IUFRF27MO2ZYU/

Its just not worth the hassle.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/667.