I am getting odd segfaults with taglib-ruby 1.1.3

[rubyFeedback]

I am getting odd segfaults with taglib-ruby 1.1.3, on Linux.

I wrote a custom wrapper in Ruby over taglib-ruby to show on the commandline
information about a .mp3 file:

audiotagreader /home/x/songs/HansZimmer_PearlHarbour_TrailerTheme.mp3

  Title:                 Pearl Harbor Trailer Theme
  Artist:                Hans Zimmer
  Album:                 
  Year:                  2001
  Genre:                 Soundtrack
  Comment:               
  Duration (in seconds): 227.5

Segmentation fault

The last one, aka Segmentation fault, seems to run when I close it or when the script ends.

Here is a partial strace result:

--- SIGVTALRM {si_signo=SIGVTALRM, si_code=SI_TKILL, si_pid=8602, si_uid=0} ---
rt_sigreturn({mask=[]})                 = 1
read(6, "", 7482)                       = 0
close(6)                                = 0
wait4(8604, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 8604
pipe2([6, 7], O_NONBLOCK|O_CLOEXEC)     = 0
pipe2([8, 9], O_NONBLOCK|O_CLOEXEC)     = 0
rt_sigprocmask(SIG_SETMASK, ~[RTMIN RT_1], [], 8) = 0
getresuid([0], [0], [0])                = 0
clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f54c7a4ba50) = 8606
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
close(9)                                = 0
fcntl(8, F_GETFL)                       = 0x800 (flags O_RDONLY|O_NONBLOCK)
fcntl(8, F_SETFL, O_RDONLY)             = 0
read(8, "", 4)                          = 0
close(8)                                = 0
close(7)                                = 0
newfstatat(6, "", {st_mode=S_IFIFO|0600, st_size=0, ...}, AT_EMPTY_PATH) = 0
read(6, 0xaf5610, 8192)                 = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=6, events=POLLIN}], 1, -1)    = 1 ([{fd=6, revents=POLLIN}])
read(6, "ffmpeg version 6.1.1 Copyright ("..., 8192) = 869
read(6, 0xaf5975, 7323)                 = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=6, events=POLLIN}], 1, -1)    = 1 ([{fd=6, revents=POLLIN}])
read(6, "[mp3 @ 0x1d865c0] Estimating dur"..., 7323) = 500
read(6, 0xaf5b69, 6823)                 = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=6, events=POLLIN}], 1, -1)    = 1 ([{fd=6, revents=POLLHUP}])
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=8606, si_uid=0, si_status=1, si_utime=0, si_stime=0} ---
write(3, "\1\0\0\0\0\0\0\0", 8)         = 8
--- SIGVTALRM {si_signo=SIGVTALRM, si_code=SI_TKILL, si_pid=8602, si_uid=0} ---
rt_sigreturn({mask=[CHLD]})             = 8
rt_sigreturn({mask=[]})                 = 1
read(6, "", 6823)                       = 0
close(6)                                = 0
wait4(8606, [{WIFEXITED(s) && WEXITSTATUS(s) == 1}], 0, NULL) = 8606
write(1, "\n", 1
)                       = 1
writev(1, [{iov_base="\33[38;2;32;178;170m  Title:      "..., iov_len=96}, {iov_base="\n", iov_len=1}], 2  Title:                 Pearl Harbor Trailer Theme
) = 97
writev(1, [{iov_base="\33[38;2;32;178;170m  Artist:     "..., iov_len=81}, {iov_base="\n", iov_len=1}], 2  Artist:                Hans Zimmer
) = 82
writev(1, [{iov_base="\33[38;2;32;178;170m  Album:      "..., iov_len=70}, {iov_base="\n", iov_len=1}], 2  Album:                 
) = 71
writev(1, [{iov_base="\33[38;2;32;178;170m  Year:       "..., iov_len=74}, {iov_base="\n", iov_len=1}], 2  Year:                  2001
) = 75
writev(1, [{iov_base="\33[38;2;32;178;170m  Genre:      "..., iov_len=80}, {iov_base="\n", iov_len=1}], 2  Genre:                 Soundtrack
) = 81
writev(1, [{iov_base="\33[38;2;32;178;170m  Comment:    "..., iov_len=70}, {iov_base="\n", iov_len=1}], 2  Comment:               
) = 71
writev(1, [{iov_base="\33[38;2;32;178;170m  Duration (in"..., iov_len=75}, {iov_base="\n", iov_len=1}], 2  Duration (in seconds): 227.5
) = 76
write(1, "\n", 1
)                       = 1
close(5)                                = 0
rt_sigaction(SIGINT, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f54c77ba3a0}, {sa_handler=0x7f54c7c5a330, sa_mask=[], sa_flags=SA_RESTORER|SA_SIGINFO, sa_restorer=0x7f54c77ba3a0}, 8) = 0
rt_sigaction(SIGINT, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f54c77ba3a0}, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f54c77ba3a0}, 8) = 0
write(3, "\1\0\0\0\0\0\0\0", 8)         = 8
munmap(0x7f54ab770000, 65536)           = 0
munmap(0x7f54ab780000, 65536)           = 0
munmap(0x7f54ab790000, 65536)           = 0
munmap(0x7f54ab7a0000, 65536)           = 0
munmap(0x7f54ab7b0000, 65536)           = 0
munmap(0x7f54ab7c0000, 65536)           = 0
munmap(0x7f54ab7d0000, 65536)           = 0
munmap(0x7f54ab7e0000, 65536)           = 0
munmap(0x7f54ab7f0000, 65536)           = 0
munmap(0x7f54ab800000, 65536)           = 0
munmap(0x7f54ab810000, 65536)           = 0
munmap(0x7f54ab820000, 65536)           = 0
munmap(0x7f54ab830000, 65536)           = 0
munmap(0x7f54ab840000, 65536)           = 0
munmap(0x7f54ab850000, 65536)           = 0
munmap(0x7f54ab860000, 65536)           = 0
munmap(0x7f54ab870000, 65536)           = 0
munmap(0x7f54ab880000, 65536)           = 0
munmap(0x7f54ab890000, 65536)           = 0
munmap(0x7f54ab8a0000, 65536)           = 0
munmap(0x7f54ab8b0000, 65536)           = 0
munmap(0x7f54ab8c0000, 65536)           = 0
munmap(0x7f54ab8d0000, 65536)           = 0
munmap(0x7f54ab8e0000, 65536)           = 0
munmap(0x7f54ab8f0000, 65536)           = 0
munmap(0x7f54ab900000, 65536)           = 0
munmap(0x7f54ab910000, 65536)           = 0
munmap(0x7f54ab920000, 65536)           = 0
munmap(0x7f54ab930000, 65536)           = 0
munmap(0x7f54ab940000, 65536)           = 0
munmap(0x7f54ab950000, 65536)           = 0
munmap(0x7f54ab9a0000, 65536)           = 0
munmap(0x7f54ab9b0000, 65536)           = 0
munmap(0x7f54ab9c0000, 65536)           = 0
munmap(0x7f54ab9d0000, 65536)           = 0
munmap(0x7f54ab9e0000, 65536)           = 0
munmap(0x7f54ab9f0000, 65536)           = 0
munmap(0x7f54aba00000, 65536)           = 0
munmap(0x7f54aba10000, 65536)           = 0
munmap(0x7f54aba20000, 65536)           = 0
munmap(0x7f54aba30000, 65536)           = 0
munmap(0x7f54aba40000, 65536)           = 0
munmap(0x7f54aba50000, 65536)           = 0
munmap(0x7f54aba60000, 65536)           = 0
munmap(0x7f54aba70000, 65536)           = 0
munmap(0x7f54aba80000, 65536)           = 0
munmap(0x7f54aba90000, 65536)           = 0
munmap(0x7f54abaa0000, 65536)           = 0
munmap(0x7f54abab0000, 65536)           = 0
munmap(0x7f54abac0000, 65536)           = 0
munmap(0x7f54abad0000, 65536)           = 0
munmap(0x7f54abae0000, 65536)           = 0
munmap(0x7f54abaf0000, 65536)           = 0
munmap(0x7f54abb00000, 65536)           = 0
munmap(0x7f54abb10000, 65536)           = 0
munmap(0x7f54abb20000, 65536)           = 0
munmap(0x7f54abb30000, 65536)           = 0
munmap(0x7f54abb40000, 65536)           = 0
munmap(0x7f54abb50000, 65536)           = 0
munmap(0x7f54abb60000, 65536)           = 0
munmap(0x7f54abb70000, 65536)           = 0
munmap(0x7f54abb80000, 65536)           = 0
munmap(0x7f54abb90000, 65536)           = 0
munmap(0x7f54abba0000, 65536)           = 0
munmap(0x7f54abbb0000, 65536)           = 0
munmap(0x7f54abbc0000, 65536)           = 0
munmap(0x7f54abbd0000, 65536)           = 0
munmap(0x7f54abbe0000, 65536)           = 0
munmap(0x7f54abbf0000, 65536)           = 0
munmap(0x7f54abc00000, 65536)           = 0
munmap(0x7f54abc10000, 65536)           = 0
munmap(0x7f54abc20000, 65536)           = 0
munmap(0x7f54abc30000, 65536)           = 0
munmap(0x7f54abc40000, 65536)           = 0
munmap(0x7f54abc50000, 65536)           = 0
munmap(0x7f54abc60000, 65536)           = 0
munmap(0x7f54abc70000, 65536)           = 0
munmap(0x7f54abc80000, 65536)           = 0
munmap(0x7f54abc90000, 65536)           = 0
munmap(0x7f54abca0000, 65536)           = 0
munmap(0x7f54abcb0000, 65536)           = 0
munmap(0x7f54abcc0000, 65536)           = 0
munmap(0x7f54abcd0000, 65536)           = 0
munmap(0x7f54abce0000, 65536)           = 0
munmap(0x7f54abcf0000, 65536)           = 0
munmap(0x7f54abd00000, 65536)           = 0
munmap(0x7f54abd10000, 65536)           = 0
munmap(0x7f54abd20000, 65536)           = 0
munmap(0x7f54abd30000, 65536)           = 0
munmap(0x7f54abd40000, 65536)           = 0
munmap(0x7f54abd50000, 65536)           = 0
munmap(0x7f54abd60000, 65536)           = 0
munmap(0x7f54abd70000, 65536)           = 0
munmap(0x7f54abd80000, 65536)           = 0
munmap(0x7f54abd90000, 65536)           = 0
munmap(0x7f54abda0000, 65536)           = 0
munmap(0x7f54abdb0000, 65536)           = 0
munmap(0x7f54abdc0000, 65536)           = 0
munmap(0x7f54abdd0000, 65536)           = 0
munmap(0x7f54abde0000, 65536)           = 0
munmap(0x7f54abdf0000, 65536)           = 0
munmap(0x7f54abe00000, 65536)           = 0
munmap(0x7f54abe10000, 65536)           = 0
munmap(0x7f54abe20000, 65536)           = 0
munmap(0x7f54abe30000, 65536)           = 0
munmap(0x7f54abe40000, 65536)           = 0
munmap(0x7f54abe50000, 65536)           = 0
munmap(0x7f54abe60000, 65536)           = 0
munmap(0x7f54abe70000, 65536)           = 0
munmap(0x7f54abe80000, 65536)           = 0
munmap(0x7f54abe90000, 65536)           = 0
munmap(0x7f54abea0000, 65536)           = 0
munmap(0x7f54abeb0000, 65536)           = 0
munmap(0x7f54abec0000, 65536)           = 0
munmap(0x7f54abed0000, 65536)           = 0
munmap(0x7f54abee0000, 65536)           = 0
munmap(0x7f54abef0000, 65536)           = 0
munmap(0x7f54abf00000, 65536)           = 0
munmap(0x7f54abf10000, 65536)           = 0
munmap(0x7f54abf20000, 65536)           = 0
munmap(0x7f54abf30000, 65536)           = 0
munmap(0x7f54abf40000, 65536)           = 0
munmap(0x7f54abf50000, 65536)           = 0
munmap(0x7f54abf60000, 65536)           = 0
munmap(0x7f54abf70000, 65536)           = 0
munmap(0x7f54abf80000, 65536)           = 0
munmap(0x7f54abf90000, 65536)           = 0
munmap(0x7f54abfa0000, 65536)           = 0
munmap(0x7f54abfb0000, 65536)           = 0
munmap(0x7f54abfc0000, 65536)           = 0
munmap(0x7f54abfd0000, 65536)           = 0
munmap(0x7f54abfe0000, 65536)           = 0
munmap(0x7f54abff0000, 65536)           = 0
munmap(0x7f54ac000000, 65536)           = 0
munmap(0x7f54ac010000, 65536)           = 0
munmap(0x7f54ac020000, 65536)           = 0
munmap(0x7f54ac030000, 65536)           = 0
munmap(0x7f54ac040000, 65536)           = 0
munmap(0x7f54ac050000, 65536)           = 0
munmap(0x7f54ac060000, 65536)           = 0
munmap(0x7f54ac070000, 65536)           = 0
munmap(0x7f54ac080000, 65536)           = 0
munmap(0x7f54ac090000, 65536)           = 0
munmap(0x7f54ac0a0000, 65536)           = 0
munmap(0x7f54ac0b0000, 65536)           = 0
munmap(0x7f54ac0d0000, 65536)           = 0
munmap(0x7f54ac0e0000, 65536)           = 0
munmap(0x7f54ac0f0000, 65536)           = 0
munmap(0x7f54ac140000, 65536)           = 0
munmap(0x7f54ac150000, 65536)           = 0
munmap(0x7f54ac160000, 65536)           = 0
munmap(0x7f54ac170000, 65536)           = 0
munmap(0x7f54ac180000, 65536)           = 0
munmap(0x7f54ac190000, 65536)           = 0
munmap(0x7f54ac1a0000, 65536)           = 0
munmap(0x7f54ac1b0000, 65536)           = 0
munmap(0x7f54ac1c0000, 65536)           = 0
munmap(0x7f54ac1d0000, 65536)           = 0
munmap(0x7f54ac1f0000, 65536)           = 0
munmap(0x7f54ac200000, 65536)           = 0
munmap(0x7f54ac210000, 65536)           = 0
munmap(0x7f54ac220000, 65536)           = 0
munmap(0x7f54ac230000, 65536)           = 0
munmap(0x7f54ac240000, 65536)           = 0
munmap(0x7f54ac250000, 65536)           = 0
munmap(0x7f54ac260000, 65536)           = 0
munmap(0x7f54ac270000, 65536)           = 0
munmap(0x7f54ac290000, 65536)           = 0
munmap(0x7f54ac2a0000, 65536)           = 0
munmap(0x7f54ac2b0000, 65536)           = 0
munmap(0x7f54ac2c0000, 65536)           = 0
munmap(0x7f54ac2d0000, 65536)           = 0
munmap(0x7f54ac2e0000, 65536)           = 0
munmap(0x7f54ac2f0000, 65536)           = 0
munmap(0x7f54ac300000, 65536)           = 0
munmap(0x7f54ac310000, 65536)           = 0
munmap(0x7f54ac320000, 65536)           = 0
munmap(0x7f54ac330000, 65536)           = 0
munmap(0x7f54ac340000, 65536)           = 0
munmap(0x7f54adfa0000, 65536)           = 0
munmap(0x7f54adfb0000, 65536)           = 0
munmap(0x7f54adfc0000, 65536)           = 0
munmap(0x7f54c73f0000, 65536)           = 0
munmap(0x7f54c7400000, 65536)           = 0
munmap(0x7f54c7410000, 65536)           = 0
munmap(0x7f54c7420000, 65536)           = 0
munmap(0x7f54c7a00000, 65536)           = 0
munmap(0x7f54c7a10000, 65536)           = 0
munmap(0x7f54c7a20000, 65536)           = 0
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=NULL} ---
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x7f54adfaab10} ---
+++ killed by SIGSEGV +++
Segmentation fault

Also, while it may not have anything to do with this, I used taglib ruby wrapper in the past in a ruby-gtk3 tag.
And I also got either random segfaults, or the application was suddenly freezing. I used to think this had to
do with some of my ruby code (hard to isolate it, this was several thousand lines of ruby code, you can see
an image of this here: https://i.imgur.com/MnKgSUF.png )

I now believe that it may be that taglib-ruby causes some issue in use.

Unfortunately I can not be more specific, so I understand this may not be super-useful to you.

However had, if you ever can revisit your own wrapper code, perhaps you can have a more closer look at
anything related to memory, as well as cleanup and finalizers and external calls. taglib-ruby is great, without
it the above GUI would not even be possible (and I intend to use this for jruby eventually one day).

Perhaps a few areas of taglib-ruby can be improved. And/or, if not possible, perhaps debug-code can be
added to indicate more easily where an issue may arise. At any rate, please feel free to close this issue -
unfortunately I can not get a better summary right now. I'll continue to experiment with this and if I
find more, I'll let you know.

[rubyFeedback]

The taglib variant I used was:

https://github.com/taglib/taglib/releases/download/v1.13.1/taglib-1.13.1.tar.gz

[rubyFeedback]

I tried it with:

https://github.com/taglib/taglib/releases/download/v2.0/taglib-2.0.tar.gz

just now, same result. So I think this segfault comes from taglib-ruby itself, not
taglib.

[robinst]

Unfortunately due to the way Ruby interacts with C++, the library is quite fragile to how binding objects are accessed/passed around, etc. And yes, there might be bugs in how the bindings are generated with regards to memory management.

Do you have any more information, such as:

• Which file types are you using (MP3, any others?)
• Does the crash always happen for the same files types (and not for other types)
• Does the crash happen at random times or is it deterministic and you can always make it happen with certain interactions
• Can you show some code of your application, just the bits that interact with taglib-ruby, to see how you're using the API?
• Which Ruby version are you using?

[lwoggardner]

I am seeing similar issue.
Ruby 3.3, 3.2
taglib 1.13.1
taglib-ruby 1.1.3 (or 1.1.2)

Fails while enumerating the memoised result of XiphComment#field_list_map , or if I remove the ||=, on the call to field_list_map itself. It seems like that object, or something within it is being freed prematurely.

All within Taglib::::File.open block, so nothing should have been freed yet.

Fails repeatedly enumerating my test library which is about 300 ogg and flac files, but not always on the same file.

    # monkey patch XiphComment to add additional getters and setters.

    class XiphComment < ::TagLib::Tag
      def tags
        @tags ||= field_list_map
      end

     def album_artist(&converter)
        #...
        tags['ALBUM_ARTIST'].map( &converter) # converter is stripping invalid filename characters
     end
end

/home/ggardner/workspace-ruby/audiofs/lib/taglib/ogg/xiph_comment.rb:26: [BUG] Segmentation fault at 0x0000000000000000
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [x86_64-linux]

-- Control frame information -----------------------------------------------
c:0031 p:---- s:0167 e:000166 CFUNC  :map
c:0030 p:0060 s:0163 e:000162 BLOCK  /home/ggardner/workspace-ruby/audiofs/lib/taglib/ogg/xiph_comment.rb:26 [FINISH]
c:0029 p:0028 s:0156 e:000155 METHOD /home/ggardner/.rvm/rubies/ruby-3.2.2/lib/ruby/3.2.0/forwardable.rb:240
c:0028 p:0077 s:0149 e:000146 BLOCK  etc/audiofs.conf.rb:56 [FINISH]
c:0027 p:0003 s:0143 e:000142 BLOCK  etc/audiofs.conf.rb:108 [FINISH]
c:0026 p:---- s:0140 e:000139 CFUNC  :instance_exec
c:0025 p:0008 s:0136 e:000135 METHOD /home/ggardner/workspace-ruby/audiofs/lib/audiofs/config.rb:31

[jacobvosmaer]

I wonder if it's the same mechanism as I theorized in #126 (comment).

Each SWIG Ruby wrapper object will free the underlying C++ object when Ruby GC's the wrapper because of the trackobjects SWIG directive. The C++ objects form a tree with *::File at its root. The File is responsible for freeing its child objects; TagLib callers should not delete child objects that still belong to a File. But Ruby GC will do that.

[lwoggardner]

Yep GC.disable avoids the issue.

So as those objects are created within the C++ wrappers and referencing each other does there need to be some kind of GC marking ? (its been a while since I did ruby C/C++ things).

Oh - the theory in that comment is that the GC of an intermediate object is explicitly destroying other objects in use rather than leaving it all to the garbage collector. Makes sense

[jacobvosmaer]

@lwoggardner thank you for framing the problem like that! I have been struggling to see a way to fix it, but indeed marking may be the solution we need.

If the parent function marks all its child objects during Ruby GC mark then that should protect us. We already have custom "free functions" in our wrapper code that do essentially the same thing (traverse C++ child objects) so each of those would need a companion "mark function".

[robinst]

+1, see https://swig.org/Doc4.3/SWIGDocumentation.html#Ruby_nn61 for an example of %markfunc in the docs

[lwoggardner]

FYI if things not being tracked/marked accurately then we should be able to reproduce the segfaults using GC.start, but I was not able to do this in a simple test case yet. Will keep trying.

[jacobvosmaer]

GC.start does not sound like it finishes GC, though. Or does it?

[lwoggardner]

At least on MRI Ruby the GC.start is just an alias for ObjectSpace.garbage_collect, which seems to default to a full and blocking event.

But now the weird thing is - if I perform a GC.start before each Taglib file open, my complex app code no longer segfaults.

[jacobvosmaer]

I wonder if the optional arguments of GC.start play a role. https://docs.ruby-lang.org/en/3.3/GC.html#method-c-start

This is particularly interesting:

The immedate_sweep keyword argument determines whether or not to defer sweeping (using lazy sweep). When set to true, sweeping is performed in steps that is interleaved with future Ruby code execution, so sweeping might not be completed during this method call. When set to false, sweeping is completed during the call to this method.

First of all note the inverted logic. When immediate_sweep is true the sweep is defered which is the opposite of what the words mean. Second of all when the sweep is deferred it is interleaved with "future code execution" meaning we have no control over when it happens.