From 0511f24c041f19c241e2a72b63600dc3e0f8d2cb Mon Sep 17 00:00:00 2001 From: Nick Sutterer Date: Mon, 27 May 2024 18:28:55 +0200 Subject: [PATCH] Use `compare_by_identity` hash for the `map` in `Circuit`. This fixes a bug that appeared with newer Ruby versions where people would use `GC.verify_compaction_references` to optimize memory consumption. After using the compaction, the circuit map would be corrupted when a task was a `Method` reference, leading to the following exception. NoMethodError: undefined method `[]' for nil /home/nick/projects/trailblazer-activity/lib/trailblazer/activity/circuit.rb:80:in `next_for' thanks to @tiagotex for deep-diving into Ruby to find where this originates from and also providing a simple solution. :wine: --- lib/trailblazer/activity/schema/compiler.rb | 2 +- test/activity_test.rb | 38 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/trailblazer/activity/schema/compiler.rb b/lib/trailblazer/activity/schema/compiler.rb index 4873719..ffa42cf 100644 --- a/lib/trailblazer/activity/schema/compiler.rb +++ b/lib/trailblazer/activity/schema/compiler.rb @@ -24,7 +24,7 @@ def call(intermediate, implementation, config_merge: {}) # From the intermediate "template" and the actual implementation, compile a {Circuit} instance. def schema_components(intermediate, implementation, config) - wiring = {} + wiring = {}.compare_by_identity # https://ruby-doc.org/3.3.0/Hash.html#class-Hash-label-Modifying+an+Active+Hash+Key nodes_attributes = [] intermediate.wiring.each do |task_ref, outs| diff --git a/test/activity_test.rb b/test/activity_test.rb index 84fbc6a..6424b2d 100644 --- a/test/activity_test.rb +++ b/test/activity_test.rb @@ -87,3 +87,41 @@ def call(*) assert_equal activity.extend(call_module).call, "overridden call!" end end + +class GCBugTest < Minitest::Spec + it "still finds {.method} tasks after GC compression" do + intermediate = Activity::Schema::Intermediate.new( + { + Activity::Schema::Intermediate::TaskRef(:a) => [Activity::Schema::Intermediate::Out(:success, :b)], + Activity::Schema::Intermediate::TaskRef(:b, stop_event: true) => [] + }, + {:b => :success}, + :a # start + ) + + module Step + extend T.def_tasks(:create) + end + + implementation = { + :a => Schema::Implementation::Task(Step.method(:create), [Activity::Output(Activity::Right, :success)], []), + :b => Schema::Implementation::Task(Trailblazer::Activity::End.new(semantic: :success), [], []), + } + + + activity = Activity.new(Activity::Schema::Intermediate::Compiler.(intermediate, implementation)) + + assert_invoke activity, seq: %([:create]) + +# TODO: add tests for different Rubys + GC.verify_compaction_references(expand_heap: true, toward: :empty) + +# Without the fix, this *might* throw the following exception: +# +# NoMethodError: undefined method `[]' for nil +# /home/nick/projects/trailblazer-activity/lib/trailblazer/activity/circuit.rb:80:in `next_for' + + assert_invoke activity, seq: %([:create]) + end + +end