Project

General

Profile

Actions

Feature #20290

closed

Add API for C extensions to free memory

Added by peterzhu2118 (Peter Zhu) 2 months ago. Updated 2 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:116902]

Description

GitHub PR: https://github.com/ruby/ruby/pull/10055

Ticket #19993 added the new feature RUBY_FREE_AT_EXIT, which frees memory in Ruby at shutdown. This allowed tools like Valgrind, ASAN, and macOS leaks to find memory leaks in Ruby without a large number of false-positives outputted. However, this feature is not complete for C extensions, as they may also need to free their memory and there was no way to do so. This means that C extensions might not be able to directly use tools like Valgrind, ASAN, or macOS leaks to find memory leaks.

This ticket proposes an API for C extensions to free memory by defining a function called Destruct_<extension name> that is called during shutdown when RUBY_FREE_AT_EXIT is enabled. This name mirrors the Init_<extension name> API that already exists for extension initialization. However, unlike the Init_<extension name> function, Destruct_<extension name> is NOT mandatory for the C extension to implement so that we can preserve backwards compatibility.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 2 months ago

I'm positive on this idea in general, but I'd like the semantics of a) what extensions can/can't do inside this function, and b) what actions the extension is expected to take, nailed down a bit.

  • Are extensions supposed to undefine the Ruby constants they defined? (Seems like no, because https://github.com/ruby/ruby/pull/10055/files#diff-d1cee85c3b0e24a64519c11150abe26fd0b5d8628a23d356dd0b535ac4595d49R4632 will walk the heap afterwards freeing things?)
  • Your implementation seems to imply to me that dfree functions for types owned by the module might still be called after Destruct_ is called, which I think is surprising?
  • Are Destruct_ methods allowed to call functions which might allocate? (looks like yes?)
  • What order are Destruct_ methods called in? I would expect inverse order of Init_ methods, but your implementation seems to walk the hash in insertion order?

Have you got an example of a particular C extension which needs to free memory in this way? That might help me understand this a bit better.

Updated by peterzhu2118 (Peter Zhu) 2 months ago

Are extensions supposed to undefine the Ruby constants they defined?

No. Any Ruby managed objects don’t need to be undefined.

Your implementation seems to imply to me that dfree functions for types owned by the module might still be called after Destruct_ is called, which I think is surprising?

I think it makes more sense for the Destruct_ function to be called after all the T_DATA objects are freed so I’ve changed the implementation.

Are Destruct_ methods allowed to call functions which might allocate? (looks like yes?)

No. Just like it is not allowed to allocate memory while freeing objects, it is not allowed (or undefined behaviour) to allocate memory in the Destruct_ functions.

What order are Destruct_ methods called in? I would expect inverse order of Init_ methods, but your implementation seems to walk the hash in insertion order?

I don’t think extensions should depend on the order in which it is called. There is a deterministic order in which it is called right now, but extensions should not depend on it.

Have you got an example of a particular C extension which needs to free memory in this way?

I haven’t looked. But if the C extension does initialization with an external library, then it might need this to release that memory at shutdown.

Updated by mdalessio (Mike Dalessio) 2 months ago · Edited

Have you got an example of a particular C extension which needs to free memory in this way?

I haven’t looked. But if the C extension does initialization with an external library, then it might need this to release that memory at shutdown.

I would consider calling libxml2's xmlCleanupParser from Nokogiri's Destruct function if this feature is made available, because libxml2 keeps some long-lived global state (like character encoding handlers) that might not otherwise be freed.

Updated by kou (Kouhei Sutou) 2 months ago

mdalessio (Mike Dalessio) wrote in #note-3:

I would consider calling libxml2's xmlCleanupParser from Nokogiri's Destruct function if this feature is made available, because libxml2 keeps some long-lived global state (like character encoding handlers) that might not otherwise be freed.

If we want to use this feature for general bound library destruction, it's better that this feature is invoked even when RUBY_FREE_AT_EXIT isn't 1.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 2 months ago

Thanks Peter, that all sounds reasonable to me.

If we want to use this feature for general bound library destruction, it's better that this feature is invoked even when RUBY_FREE_AT_EXIT isn't 1.

Hm. I wonder if extension developers should themselves check if RUBY_FREE_AT_EXIT is set when deciding what to do in their Destruct_ functions. Obviously they shouldn't be calling getenv and doing this parsing themselves, but maybe a parameter or function can be exposed where developers can do "important" cleanup unconditionally, but "optional" cleanup only if memory leak checking is on.

I don't have a strong idea about what's "important" vs "optional" though.

Updated by shyouhei (Shyouhei Urabe) 2 months ago

There already is ruby_vm_at_exit(). What's wrong with that?

Updated by peterzhu2118 (Peter Zhu) 2 months ago

There already is ruby_vm_at_exit(). What's wrong with that?

I didn't know that ruby_vm_at_exit exists. I think ruby_vm_at_exit can do everything in this feature. I might close this ticket in that case.

Updated by peterzhu2118 (Peter Zhu) 2 months ago

  • Status changed from Open to Closed

I opened #20306 to succeed this ticket.

Actions

Also available in: Atom PDF

Like2
Like0Like0Like1Like0Like0Like0Like0Like0