Backport #8531: ifuncに渡したブロックが共有される

• Status: Closed
• Assignee: usa (Usaku NAKAMURA)

[ktsj (Kazuki Tsujimoto)]

=begin
ifunc(rb_iterateでbl_procとして渡したもの)をブロック付きで呼び出すと、
渡したブロックがifuncのフレーム内に保存されるようになっていますが(r29335)、

2072 vm_yield_with_cfunc(rb_thread_t *th, const rb_block_t *block,
....
2107 if (blockargptr) {
2108 VM_CF_LEP(cfp)[0] = VM_ENVVAL_BLOCK_PTR(blockargptr);
2109 }

これによりメソッドに渡したブロックが次回のメソッド呼び出し時にもそのままフレームに残り続けて
共有されてしまうことがあります。#8341でMethod#to_procの件が報告されていますが、
Symbol#to_procなどでも同様です。

c = Class.new do
def foo
if block_given?
yield
else
puts "No block given."
end
end
end

o = c.new
f = :foo.to_proc
f.(o) { puts "Block given." }

=> Block given.

f.(o)

=> Block given.

特に明文化されていないですが、ifuncには渡されたブロックを参照するのにLEPが利用できず(PASS_PASSED_BLOCKなどが使えない)、
引数として渡されるblockargを使わなければならないという制約があるものと思っています。
(正確に言えば利用できないわけではなくて、RubyレベルでいうProc内でのblock_given?などと同等の動きになる)

現在フレームにブロックを保存するようしているのはこの制約を回避してifuncからrb_method_callなどを期待通りに呼び出すためですが、
ブロックを共有してしまうという弊害がある以上ブロックは引数で渡すという形に修正するのが妥当ではないでしょうか。
この考え方で作ったSymbol#to_procの修正パッチを添付します。(Method#to_procについては#8341に添付しています)

なお、今の公開APIには任意のProcオブジェクトをpassed_blockとしてメソッドを呼び出すための関数がなさそうなので
利便性のために追加したりしていますが、この辺りは議論が必要そうな気がします。
=end

[nobu (Nobuyoshi Nakada)]

• Assignee changed from ko1 (Koichi Sasada) to ktsj (Kazuki Tsujimoto)

いいんじゃないでしょうか。

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r41343.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

test/ruby/test_symbol.rb: tests for [Bug #8531]

[nobu (Nobuyoshi Nakada)]

• Status changed from Closed to Assigned
• % Done changed from 100 to 0
• Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED

[ktsj (Kazuki Tsujimoto)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r41360.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• include/ruby/ruby.h, vm_eval.c (rb_funcall_with_block):
  new function to invoke a method with a block passed
  as an argument.

• string.c (sym_call): use the above function to avoid
  a block sharing. [ruby-dev:47438] [Bug #8531]

• vm_insnhelper.c (vm_yield_with_cfunc): don't set block
  in the frame.

• test/ruby/test_symbol.rb (TestSymbol#test_block_given_to_proc):
  run related tests.

[ktsj (Kazuki Tsujimoto)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport200
• Status changed from Closed to Assigned
• Assignee changed from ktsj (Kazuki Tsujimoto) to nagachika (Tomoyuki Chikanaga)
• Target version deleted (2.1.0)

Please backport r41343, r41360.

[ktsj (Kazuki Tsujimoto)]

＞ なかださん
追加していただいた TestSymbol#test_block_persist_between_calls ですが、
skipを外すとテストに失敗します。

テスト側のバグのように思うのですが、確認していただけないでしょうか。

[nagachika (Tomoyuki Chikanaga)]

• Tracker changed from Backport to Bug
• Project changed from Backport200 to Ruby master
• Assignee changed from nagachika (Tomoyuki Chikanaga) to nobu (Nobuyoshi Nakada)

テストに残りの修正があるみたいなので一旦戻しておきますね。

[nobu (Nobuyoshi Nakada)]

• Status changed from Assigned to Closed

This issue was solved with changeset r41386.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

test_symbol.rb: fix test

• test/ruby/test_symbol.rb (test_block_persist_between_calls): needs
  receiver object. [Bug #8531]

[nagachika (Tomoyuki Chikanaga)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport200
• Status changed from Closed to Assigned
• Assignee changed from nobu (Nobuyoshi Nakada) to nagachika (Tomoyuki Chikanaga)

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Assigned to Closed

This issue was solved with changeset r41393.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 41343,41360,41386: [Backport #8531]

test/ruby/test_symbol.rb: tests for [Bug #8531]
* include/ruby/ruby.h, vm_eval.c (rb_funcall_with_block):
  new function to invoke a method with a block passed
  as an argument.

* string.c (sym_call): use the above function to avoid
  a block sharing. [ruby-dev:47438] [Bug #8531]

* vm_insnhelper.c (vm_yield_with_cfunc): don't set block
  in the frame.

* test/ruby/test_symbol.rb (TestSymbol#test_block_given_to_proc):
  run related tests.

[nagachika (Tomoyuki Chikanaga)]

• Project changed from Backport200 to Backport193
• Status changed from Closed to Assigned
• Assignee changed from nagachika (Tomoyuki Chikanaga) to usa (Usaku NAKAMURA)

[usa (Usaku NAKAMURA)]

• Status changed from Assigned to Closed

This issue was solved with changeset r41653.
Kazuki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

merge revision(s) 41343,41360,41386: [Backport #8531]

test/ruby/test_symbol.rb: tests for [Bug #8531]
* include/ruby/ruby.h, vm_eval.c (rb_funcall_with_block):
  new function to invoke a method with a block passed
  as an argument.

* string.c (sym_call): use the above function to avoid
  a block sharing. [ruby-dev:47438] [Bug #8531]

* vm_insnhelper.c (vm_yield_with_cfunc): don't set block
  in the frame.

* test/ruby/test_symbol.rb (TestSymbol#test_block_given_to_proc):
  run related tests.