Skip to content

Conversation

@daipom
Copy link
Contributor

@daipom daipom commented Mar 5, 2025

Which issue(s) this PR fixes:

What this PR does / why we need it:
Sigdump.dump iterates ObjectSpace.
If there are DoubleDefinition objects of RR, it can set some expectations of RR unexpectedly.
It can cause test failure.

By doing GC, we can remove DoubleDefinition objects made in other tests.

See #4063

Failure: test_cont_in_main_process_signal_handlers(SupervisorTest):
  class()
  Called 0 times.
  Expected 1 times.

(...)/sigdump-0.2.5/lib/sigdump.rb:76:in block in dump_object_count' (...)/sigdump-0.2.5/lib/sigdump.rb:75:in each_object'
(...)/sigdump-0.2.5/lib/sigdump.rb:75:in dump_object_count' (...)/sigdump-0.2.5/lib/sigdump.rb:18:in block in dump'
(...)/sigdump-0.2.5/lib/sigdump.rb:138:in open' (...)/sigdump-0.2.5/lib/sigdump.rb:138:in _open_dump_path'
(...)/sigdump-0.2.5/lib/sigdump.rb:14:in dump' (...)/fluentd/fluentd/lib/fluent/supervisor.rb:1097:in dump_non_windows'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:997:in block in install_main_process_signal_handlers' (...)/fluentd/fluentd/test/test_supervisor.rb:270:in kill'
(...)/fluentd/fluentd/test/test_supervisor.rb:270:in `test_cont_in_main_process_signal_handlers'
267: sv = Fluent::Supervisor.new({})
268: sv.send(:install_main_process_signal_handlers)
269:
=> 270: Process.kill :CONT, Process.pid
271:
272: sleep 1
273:

Docs Changes:
Not needed.

Release Note:
Not needed.

**Which issue(s) this PR fixes**:
* Fixes #4063

**What this PR does / why we need it**:
Sigdump.dump iterates ObjectSpace.
If there are DoubleDefinition objects of RR, it can set some
expectations of RR unexpectedly.
It can cause test failure.

By doing GC, we can remove DoubleDefinition objects made in other tests.

See #4063

    Failure: test_cont_in_main_process_signal_handlers(SupervisorTest):
      class()
      Called 0 times.
      Expected 1 times.
(...)/sigdump-0.2.5/lib/sigdump.rb:76:in `block in dump_object_count'
    (...)/sigdump-0.2.5/lib/sigdump.rb:75:in `each_object'
    (...)/sigdump-0.2.5/lib/sigdump.rb:75:in `dump_object_count'
    (...)/sigdump-0.2.5/lib/sigdump.rb:18:in `block in dump'
    (...)/sigdump-0.2.5/lib/sigdump.rb:138:in `open'
    (...)/sigdump-0.2.5/lib/sigdump.rb:138:in `_open_dump_path'
    (...)/sigdump-0.2.5/lib/sigdump.rb:14:in `dump'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:1097:in
`dump_non_windows'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:997:in `block in
install_main_process_signal_handlers'
    (...)/fluentd/fluentd/test/test_supervisor.rb:270:in `kill'
(...)/fluentd/fluentd/test/test_supervisor.rb:270:in
`test_cont_in_main_process_signal_handlers'
         267:     sv = Fluent::Supervisor.new({})
         268:     sv.send(:install_main_process_signal_handlers)
         269:
      => 270:     Process.kill :CONT, Process.pid
         271:
         272:     sleep 1
         273:

**Docs Changes**:
Not needed.

**Release Note**:
Not needed.

Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom daipom added the CI Test/CI issues label Mar 5, 2025
@daipom daipom added this to the v1.16.8 milestone Mar 5, 2025
Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@kenhys kenhys merged commit 2caaec9 into v1.16 Mar 5, 2025
15 of 16 checks passed
@kenhys kenhys deleted the backport-4846 branch March 5, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Test/CI issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants