Jens-G opened a new pull request, #3934: URL: https://github.com/apache/thrift/pull/3934
The `finalize` function the c_glib generator writes for structs and exceptions, including the argument and result structs of every service method, freed the members but never called the parent class's `finalize`. GObject's own `finalize` therefore never ran for these objects, and that is the step that releases the instance data (`g_datalist_clear()`): data attached with `g_object_set_data_full()`, and what `g_object_unref()` stores there itself while it disposes of an object. With GLib 2.72.4, valgrind shows 48 bytes lost for every generated object that is created and destroyed; with this change, none. Eight hand-written classes in `lib/c_glib` had the same gap: `ThriftApplicationException`, `ThriftBufferedTransport`, `ThriftFDTransport`, `ThriftFramedTransport`, `ThriftMemoryBuffer`, `ThriftServerSocket`, `ThriftSocket` and `ThriftZlibTransport`. `ThriftSSLSocket` chains up, but into `ThriftSocket`'s `finalize`, so it was affected too. - **Generator:** the generated `class_init` stores the parent class in a `static gpointer <type>_parent_class` (the name `G_DEFINE_TYPE` would give it), and the generated `finalize` ends with `G_OBJECT_CLASS (<type>_parent_class)->finalize (object);`. The generated output only gains these three lines per type. - **Library:** the eight `finalize` functions chain up as their last step. The ticket's workaround, a `g_datalist_clear()` in the generated `finalize`, would release the data, but it repeats one step of `g_object_finalize()` instead of running the parent chain, and it does not cover the library classes. ### How it was checked - A new test, `lib/c_glib/test/testfinalize.c` (registered for CMake and autotools), covers a generated struct and exception and the seven classes of the core library; the zlib transport case is in `testzlibtransport.c`, which is only built with zlib support. Each case attaches data with a destroy notification, drops the last reference and checks that the notification ran. They fail on the current code and pass with the change. - CMake (`WITH_CPP=ON -DBUILD_CPP=OFF`, so that zlib is found): all 27 c_glib tests pass. Autotools, `make -C lib/c_glib/test check-am`: 32 of 32. Both with GLib 2.72.4 on Ubuntu 22.04. - valgrind: all 27 c_glib test binaries run clean; a probe that creates and destroys a generated struct 1,000 times lost 48,000 bytes before the change and nothing after. - `git clang-format` is clean on the generator change; cppcheck 2.7 with the SCA job's options and gcc `-Wall -Wextra` report nothing new. Under autotools, `testzlibtransport` also needs the one-line link fix on `Jens-G:c_glib-zlib-test-link`: since `cbfb46b7f`, `testzlibtransport_LDADD` on master lacks the memory buffer object. CMake is not affected. No CI job runs the c_glib tests yet; THRIFT-6361 is about adding one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
