10

As every Ruby programmer eventually discovers, calling blocks or procs that contain return statements can be dangerous as this might exit your current context:

def some_method(&_block)
   puts 1
   yield
   # The following line will never be executed in this example
   # as the yield is actually a `yield-and-return`.
   puts 3
end

def test
  some_method do
    puts 2
    return
  end
end

test

# This prints "1\n2\n" instead of "1\n2\n3\n"    

In cases you want to be absolutely sure some of your code runs after you called a block or proc, you can use a begin ... ensure construct. But since ensure is also called if there is an exception during yield, it requires a little more work.

I've created a tiny module that deals with this problem in two different ways:

  1. Using safe_yield, it is detected whether the yielded block or proc actually returns using the return keyword. If so, it raises an exception.

    unknown_block = proc do
      return
    end 
    
    ReturnSafeYield.safe_yield(unknown_block)
    # => Raises a UnexpectedReturnException exception
    
  2. Using call_then_yield, you can call a block and then make sure that a second block is executed, even if the first block contains a return statement.

    unknown_block = proc do
      return
    end
    ReturnSafeYield.call_then_yield(unknown_block) do
      # => This line is called even though the above block contains a `return`.
    end
    

I'm considering to create a quick Gem out of this, or is there any built-in solution to prevent quick return from the nested block which I missed?

user513951
  • 12,445
  • 7
  • 65
  • 82
sudoremo
  • 2,274
  • 2
  • 22
  • 39
  • Just added a little example showing the problem. The basic issue only arises when you're not in control of the block / proc you're yielding (i.e. when writing a library). – sudoremo Dec 12 '16 at 12:42
  • 7
    This will break a default ruby behavior, bringing more pain, than profit. Imagine I am the consumer of the code that uses this trick. As I put `return` inside my block, I expect it to pass control immediately, and I would be damn surprised that some weird exception was raised. – Aleksei Matiushkin Dec 12 '16 at 12:43
  • 5
    Covering hunting pits with a hay only hides a trap, making code harder to debug. Ruby is not a language to protect people from shooting their legs, and this is the main advantage of it. – Aleksei Matiushkin Dec 12 '16 at 12:47
  • 2
    This is not a good idea, but it _is_ a good question. Thank you for asking an interesting question. – Wayne Conrad Dec 12 '16 at 12:49
  • 2
    Why would you pass (or even create) a proc containing a `return` statement in the first place? – Stefan Dec 12 '16 at 12:51
  • 1
    @Stefan the question, AFAIU, is about how to protect _our_ code, that `yield`s, from the dumb consumer, who calls `return` in the block leading to _us_ losing the control. BTW, `return` from a block is a dangerous but effective stack unwind practice in some limited number of cases (e.g. in recursive block calls :) – Aleksei Matiushkin Dec 12 '16 at 13:00
  • _Sidenote:_ the code you linked does not support block arguments, what after all makes it completely useless. – Aleksei Matiushkin Dec 12 '16 at 13:04
  • 1
    @mudasobwa I'd use `ensure` in that case, regardless of the cause. Regarding the limited use cases – this only works for block created within the method. You can't pass a `return` to a method from the outside, can you? – Stefan Dec 12 '16 at 13:24
  • @Stefan I am not sure I understood the last sentence, could you please explore? One can [technically] pass a `return` to a method from the outside via the `block` having `return`. In such a case `yield` [technically] becomes yield-and-return :) – Aleksei Matiushkin Dec 12 '16 at 13:31
  • @mudasobwa really? `def foo; yield; end; foo { return }` results in a `LocalJumpError` – Stefan Dec 12 '16 at 13:33
  • 1
    @Stefan: Yes: `def foo; yield; end; def main; foo { return }; end`. @mudasobwa: Exactly, the point of this approach is to protect our library code from possible programming mistakes out there. Good point with block arguments, will attempt to fix that. – sudoremo Dec 12 '16 at 13:35
  • Method `safe_yield` now supports block arguments. Thanks @mudasobwa for the hint. – sudoremo Dec 12 '16 at 13:37
  • Just to be clear: These methods are *not* meant for standard cases, but in very specific applications they might be useful. For instance, we're writing some kind of a transaction wrapping library that can be called with something like `Tx.t { ... }`. Inside such a block, a `return` is just wrong and would lead to unexpected behavior, so the `begin ... rescue` construct is an absolute necessity there. And if it gets a little more complicated, I tend to hide such non-business-logic complexity in external modules such as this one. – sudoremo Dec 12 '16 at 13:41
  • @Remo the blocks accept block as block argument (argh :) and the block _is not accessible_ via `args`. That said, you should add the block argument `self.safe_yield(block, *args, &cb)` to your methods as well. – Aleksei Matiushkin Dec 12 '16 at 13:45
  • 1
    @Remo I still think `ensure` is the right way here. Why would you want to `puts 3` in case of a `return` but not in case of an error / exception. Why should your code care about what's happening within yield? – Stefan Dec 12 '16 at 14:07
  • @Stefan: Think of something very simple: `state = :running; yield; state = :success;`. In this example, your library code might be stuck in state `running` if the consumer is not careful. If you're just coding without thinking about the return, you wouldn't need an `ensure` for this case but could just write it as above, maybe wrap a `begin ... rescue` around it to set the state to `exception` or what not. And using `safe_yield`, you can do just that without having to worry about dangerous use cases by the consumer of your library. – sudoremo Dec 12 '16 at 14:10
  • 5
    Me, I believe that users should be totally allowed to shoot themselves in the foot. – Sergio Tulentsev Dec 12 '16 at 14:19
  • 1
    @Remo (1) If your code took a lambda rather than a block/proc, then `return` in the passed-in code would only return from the lambda. (2) Have you considered posting this question on http://codereview.stackexchange.com ? – Wayne Conrad Dec 12 '16 at 14:21
  • 1
    @mudasobwa, I don't think there are legitimate use cases, considering that you have `try`/`catch` if you want to do crazy jumps through the stack. – ndnenkov Dec 12 '16 at 15:39
  • 1
    If you really, really, **really** want code to run, use `ensure`. When you call `yield` you're briefly surrendering control to some other block of code and you need to be able to live with the consequences. – tadman Dec 12 '16 at 17:21
  • oooooo, this makes my skin crawl. We're OK if we get around `return`, but what if block also contains `launch_missiles(:now)`? Surely untrusted code cannot be allowed to pass a block or proc (e.g., raise an exception if `block_given?`), and we may need to carefully examine the arguments being passed before blindly executing the method. – Cary Swoveland Dec 12 '16 at 18:10
  • @CarySwoveland I totally get your point. I agree that if the "block author" wants to blow up the world, it should. But the point is a different one: It was an active decision to launch the missiles. It is however totally unknown to the "block author" what happens if `return` is used instead. We can't protect API users from every mistake, but we can protect it from some mistakes that would otherwise need detailed knowledge of an API's internal code. – sudoremo Dec 13 '16 at 10:38
  • 1
    Thanks a bunch for your valuable feedback. I get that there is quite some 'resistance', but we have a situation where we really need a proper solution for this. So I took the liberty of creating a Gem anyway (https://github.com/remofritzsche/return_safe_yield). I've stated in the readme that this is a contraversal Gem. Please let me know if you have any suggestions for the Gem or the readme. – sudoremo Dec 13 '16 at 10:40

1 Answers1

3

There is a built-in solution to detect whether a block contains a return statement.

You can use RubyVM::InstructionSequence.disasm to disassemble the block passed in by the user, then search it for throw 1, which represents a return statement.

Here's a sample implementation:

def safe_yield(&block)
  if RubyVM::InstructionSequence.disasm(block) =~ /^\d+ throw +1$/
    raise LocalJumpError
  end

  block.call
end

Here's how you might incorporate it into your library:

def library_method(&block)
  safe_yield(&block)
  puts "library_method succeeded"
rescue LocalJumpError
  puts "library_method encountered illegal return but resumed execution"
end

And here's the user experience for a well-behaved and a misbehaving user:

def nice_user_method
  library_method { 1 + 1 }
end

nice_user_method
# library_method succeeded

def naughty_user_method
  library_method { return false if rand > 0.5 }
end

naughty_user_method
# library_method encountered illegal return but resumed execution

Commentary:

Using raise LocalJumpError/rescue LocalJumpError gets around the issues you encountered when using a blanket ensure.

I chose LocalJumpError because it seems relevant, and because (I think!) there is no possible Ruby code that would result in LocalJumpError being raised "naturally" in this context. If that turns out to be false, you can easily substitute your own new exception class.

user513951
  • 12,445
  • 7
  • 65
  • 82