daveallie / entangler

Two way file syncer using platform native notify
MIT License
8 stars 6 forks source link

Add missing `ensure` #5

Closed ghiculescu closed 3 years ago

ghiculescu commented 3 years ago

The return here is problematic. It can put entangler into a state where @listener_pauses[1] is always true, which can cause it to be permanently paused. This is because if you return from inside a block's yield, anything after the yield won't run. ensure exists to protect against this. This script demonstrates the problem:

def with_foo_bad(&block)
  puts "start doing things"
  yield
  puts "after doing things"
end

def with_foo_good(&block)
  puts "start doing things"
  yield
  ensure
    puts "after doing things"
end

def wrapped_bad_method_1
  with_foo_bad do
    puts "inside"
  end
end

def wrapped_bad_method_2
  with_foo_bad do
    return
  end
end

def wrapped_good_method_1
  with_foo_good do
    puts "inside"
  end
end

def wrapped_good_method_2
  with_foo_good do
    return
  end
end

puts "start wrapped_bad_method_1"
wrapped_bad_method_1
puts ""

puts "start wrapped_bad_method_2"
wrapped_bad_method_2
puts ""

puts "start wrapped_good_method_1"
wrapped_good_method_1
puts ""

puts "start wrapped_good_method_2"
wrapped_good_method_2
puts ""
% ruby test.rb
start wrapped_bad_method_1
start doing things
inside
after doing things

start wrapped_bad_method_2
start doing things # note that "after doing things" is never printed

start wrapped_good_method_1
start doing things
inside
after doing things

start wrapped_good_method_2
start doing things
after doing things # this is good
hey-leon commented 3 years ago

👋 @daveallie. hope you are well 🤣