Skip to content

Commit

Permalink
Use compare_by_identity hash for the map in Circuit. This fixes…
Browse files Browse the repository at this point in the history
… 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:
  • Loading branch information
apotonick committed Oct 8, 2024
1 parent f2dedc7 commit 0511f24
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/trailblazer/activity/schema/compiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
38 changes: 38 additions & 0 deletions test/activity_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 0511f24

Please sign in to comment.