Backport #8531

ifuncに渡したブロックが共有される

Added by Kazuki Tsujimoto 10 months ago. Updated 10 months ago.

[ruby-dev:47438]
Status:Closed
Priority:Normal
Assignee:Usaku NAKAMURA

Description

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

2072 vmyieldwithcfunc(rbthreadt *th, const rbblockt *block,
....
2107 if (blockargptr) {
2108 VM
CFLEP(cfp)[0] = VMENVVALBLOCKPTR(blockargptr);
2109 }

これによりメソッドに渡したブロックが次回のメソッド呼び出し時にもそのままフレームに残り続けて
共有されてしまうことがあります。#8341でMethod#toprocの件が報告されていますが、
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が利用できず(PASSPASSEDBLOCKなどが使えない)、
引数として渡されるblockargを使わなければならないという制約があるものと思っています。
(正確に言えば利用できないわけではなくて、RubyレベルでいうProc内でのblock_given?などと同等の動きになる)

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

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

fix-ifunc-block.patch Magnifier (2.8 KB) Kazuki Tsujimoto, 06/16/2013 03:33 AM


Related issues

Related to Backport93 - Backport #8341: block_given? (and the actual block) persist between calls... Closed 04/28/2013

Associated revisions

Revision 41653
Added by Usaku NAKAMURA 10 months ago

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.  [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.

History

#1 Updated by Nobuyoshi Nakada 10 months ago

  • Assignee changed from Koichi Sasada to Kazuki Tsujimoto

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

#2 Updated by Nobuyoshi Nakada 10 months ago

  • 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]

#3 Updated by Nobuyoshi Nakada 10 months ago

  • 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

#4 Updated by Kazuki Tsujimoto 10 months ago

  • 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, vmeval.c (rbfuncallwithblock):
    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. [Bug #8531]

  • vminsnhelper.c (vmyieldwithcfunc): don't set block
    in the frame.

  • test/ruby/testsymbol.rb (TestSymbol#testblockgivento_proc):
    run related tests.

#5 Updated by Kazuki Tsujimoto 10 months ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport200
  • Status changed from Closed to Assigned
  • Assignee changed from Kazuki Tsujimoto to Tomoyuki Chikanaga
  • Target version deleted (2.1.0)

Please backport r41343, r41360.

#6 Updated by Kazuki Tsujimoto 10 months ago

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

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

#7 Updated by Tomoyuki Chikanaga 10 months ago

  • Tracker changed from Backport to Bug
  • Project changed from Backport200 to ruby-trunk
  • Assignee changed from Tomoyuki Chikanaga to Nobuyoshi Nakada

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

#8 Updated by Nobuyoshi Nakada 10 months ago

  • 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/testsymbol.rb (testblockpersistbetween_calls): needs receiver object. [Bug #8531]

#9 Updated by Tomoyuki Chikanaga 10 months ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport200
  • Status changed from Closed to Assigned
  • Assignee changed from Nobuyoshi Nakada to Tomoyuki Chikanaga

#10 Updated by Tomoyuki Chikanaga 10 months ago

  • 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.  [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.

#11 Updated by Tomoyuki Chikanaga 10 months ago

  • Project changed from Backport200 to Backport93
  • Status changed from Closed to Assigned
  • Assignee changed from Tomoyuki Chikanaga to Usaku NAKAMURA

#12 Updated by Usaku NAKAMURA 10 months ago

  • 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.  [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.

Also available in: Atom PDF