Project

General

Profile

Actions

Bug #20718

closed

Objects created with Data_Make_Struct and the default free function are not freed

Added by jcalvert (Jonathan Calvert) 4 months ago. Updated about 2 months ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.3.5 (2024-09-03 revision ef084cc8f4) [x86_64-linux]
[ruby-core:119091]

Description

I discovered a memory leak when using the FFI gem prior to version 1.16 and Ruby 3.3 and up.

During debugging I found that this earlier version of FFI uses Data_Make_Struct (https://github.com/ffi/ffi/blob/v1.15.5/ext/ffi_c/Pointer.c#L57) instead of TypedData_Make_Struct and it uses -1 as the free function, which is RUBY_DEFAULT_FREE

When the object goes to get garbage collected, it enters into rb_data_free and it is passed to the RTYPEDDATA_EMBEDDED_P macro even though it is not of RTypedData. Because of that, the conditional is evaluated to false and xfree is never called. This was discovered by using jemalloc leak detection.

I have attached a somewhat minimal replication of the issue. The fix would appear to check the type of the obj before casting it.


Files

pointer_bug.rb (418 Bytes) pointer_bug.rb replication using ffi gem jcalvert (Jonathan Calvert), 09/06/2024 09:40 PM
Gemfile.txt (104 Bytes) Gemfile.txt gemfile for convenience jcalvert (Jonathan Calvert), 09/06/2024 09:41 PM

Updated by jcalvert (Jonathan Calvert) 4 months ago ยท Edited

I have added a pull request that should patch the issue. https://github.com/ruby/ruby/pull/11563 - I was able to build Ruby and run it against my example and it does work.

Presumably this would be a candidate for a backport :)

Actions #2

Updated by jcalvert (Jonathan Calvert) 4 months ago

  • Status changed from Open to Closed

Applied in changeset git|c1a510a8dffa1c8065e47697cd57edae67126712.


[Bug #20718] Free non-RTypedData objects

Allow objects that are not of type RTypedData to use the default
free function, as RTYPEDDATA_EMBEDDED_P can return a false positive
when casting non-RTypedData objects.

Updated by byroot (Jean Boussier) 4 months ago

  • Backport changed from 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.1: DONTNEED, 3.2: DONTNEED, 3.3: REQUIRED

I think this issue was introduced in 3.3? Let me know if not and I'll update the backport target.

Also for 3.3 the branch maintainer appreciate backport PRs so don't hesitate to open it yourself and tag k0kubun if you wish.

Updated by k0kubun (Takashi Kokubun) about 2 months ago

  • Backport changed from 3.1: DONTNEED, 3.2: DONTNEED, 3.3: REQUIRED to 3.1: DONTNEED, 3.2: DONTNEED, 3.3: DONE
Actions

Also available in: Atom PDF

Like0
Like0Like0Like1Like0