Skip to content

Commit 9406ad2

Browse files
sisshiki1969claude
andauthored
Enumerator: Yielder#yield returns nil; Yielder inherits Object (fixes #905) (#907)
Two bugs made `r = []; Enumerator.new { |y| r << y.yield(1) }.to_a; p r` abort the process (non-unwinding panic in `Value::as_array`): 1. The generator driver `generator_yield_values` resumed the fiber with the `yielder` on *every* activation. The yielder is only the block parameter on the first (Created) activation; on later resumes that value becomes the return value of the `y.yield(...)` that suspended, so `Yielder#yield` returned the Yielder itself instead of nil (CRuby returns nil for each/to_a). That leaked a Yielder into user data. Resume with `Value::nil()`. 2. `Enumerator::Yielder` was defined as a subclass of `Array`, but a yielder is a plain OBJECT-typed RValue. Inherited `Array#inspect` (etc.) then ran `Value::as_array` on a non-array and hit the assertion, aborting the process the moment such a leaked Yielder was inspected. Make `Yielder` inherit `Object`, as in CRuby (`Enumerator::Yielder.superclass == Object`); its `<<` / `yield` are defined directly. With both fixes the repro prints `[nil]` and `Yielder#yield` returns nil, matching CRuby. core/enumerator/new_spec no longer crashes and its `#yield returns nil` example passes, so its spec/tags entry is removed. Full lib suite: 1828 passed. Claude-Session: https://claude.ai/code/session_01XpzdoFu46d2ChJpSzeWsNK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 498118c commit 9406ad2

4 files changed

Lines changed: 58 additions & 16 deletions

File tree

doc/ruby_spec_skip_tags.md

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -186,30 +186,33 @@ Mutex/Queue/Thread の状態)を残し、*後続*ファイルをハングさ
186186
crash/hang の原因は *特定の1〜数 example* に限られることを突き止め、その example
187187
だけを `fails:` タグで除外すれば残りは完走することを確認したため、すべて tags へ
188188
移行した。
189-
- `spec/tags/`: **11 ファイル**。各ファイルで crash/hang する example のみ除外:
189+
- `spec/tags/`: **10 ファイル**。各ファイルで crash/hang する example のみ除外:
190190
- I/O 系ブロッキング(tag 適用で fast-path 完走): `core/argf/read`
191191
`/dev/zero` 無限読み)、`core/argf/readlines``core/io/copy_stream`
192192
`core/io/select`(2件)、`library/socket/socket/{tcp,udp}_server_loop`
193193
`library/expect/expect``IO#expect` がブロックする4件のみ。閉じた IO / EOF の
194194
2件は即返るので除外しない)。
195-
- CRASH / プロセス死(1 example ずつ):
196-
- `core/enumerator/new``Enumerator.new when passed a block #yield returns nil`
197-
`Yielder#yield` の戻り値を使うと `Value::as_array``value.rs:2272`)で非巻き
198-
戻し panic → プロセス abort。**本体バグ(issue #905)**。tag は暫定回避で、
199-
根治すれば tag を外して13 example を復帰できる。
200-
- `core/exception/{signal_exception,signm,signo}`
201-
`Process.kill(:TERM, Process.pid)` を実行する example。monoruby は SIGTERM を
202-
rescue 可能な `SignalException` に変換しないためプロセスが終了する。
203-
`signal_exception` は他15 example が復帰。`signm`/`signo` は example が1件だけ
204-
なので実効は skip と同じ(tag で統一)。
195+
- プロセス死(1 example ずつ): `core/exception/{signal_exception,signm,signo}`
196+
`Process.kill(:TERM, Process.pid)` を実行する example。monoruby は SIGTERM を
197+
rescue 可能な `SignalException` に変換しないためプロセスが終了する。
198+
`signal_exception` は他15 example が復帰。`signm`/`signo` は example が1件だけ
199+
なので実効は skip と同じ(tag で統一)。
200+
- (`core/enumerator/new` は issue #905 の `as_array` panic を根治したため tag を
201+
撤去。`#yield returns nil` は現在 pass する。)
205202
- CI 側で tags のコピー先を `spec/tags/ruby/` に修正済み(上部「更新」参照)なので
206203
`mspec ci` がタグを読み、crash/hang する example だけ除外して fast-path が完走する。
207204
- `library/cgi/unescapeURIComponent` は報告にあったが実際はハングしない
208205
(約0.35秒)ので対象外。
209206
210207
## フォローアップ
211208
212-
- **`core/enumerator/new``as_array` panic**(issue #905)を根治し、tag を外す。
209+
- ~~**`core/enumerator/new``as_array` panic**(issue #905)~~ — **修正済み**。
210+
真因は2点: (1) generator の駆動(`generator_yield_values`)が resume 時にも
211+
`yielder` を渡していたため `Yielder#yield` が nil ではなく Yielder 自身を返し、
212+
ユーザ側(`r << y.yield(1)`)へ Yielder が漏れていた。(2) `Yielder``Array`
213+
サブクラスで、継承した `Array#inspect` 等が非配列の Yielder に `Value::as_array`
214+
を実行して非巻き戻し panic → abort。resume 値を `nil` にし、`Yielder` の親クラスを
215+
`Object` に変更(CRuby 準拠)。tag は撤去し `#yield returns nil` は pass する。
213216
- **SIGTERM → `SignalException` 変換**(signal handling の実装)を入れれば
214217
`signal_exception`/`signm`/`signo` の該当 example と tag を外せる。
215218
- ~~**`core/io` の fd 二重クローズ crash**~~ — **修正済み**。原因は

monoruby/src/builtins/enumerator.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,13 @@ pub(super) fn init(globals: &mut Globals) {
2929
globals.define_builtin_func(ENUMERATOR_CLASS, "rewind", rewind, 0);
3030
globals.define_builtin_funcs(ENUMERATOR_CLASS, "size", &["length"], enumerator_size, 0);
3131

32-
let array_class = globals[ARRAY_CLASS].get_module();
33-
let yielder = globals.define_class("Yielder", array_class, ENUMERATOR_CLASS);
32+
// `Enumerator::Yielder` inherits from `Object` (as in CRuby), NOT `Array`.
33+
// The yielder object is a plain OBJECT-typed RValue; inheriting `Array`
34+
// meant inherited methods like `Array#inspect` ran `Value::as_array` on a
35+
// non-array and aborted the process (e.g. `p` on a value that had leaked a
36+
// Yielder). Its `<<` / `yield` are defined directly below.
37+
let object_class = globals[OBJECT_CLASS].get_module();
38+
let yielder = globals.define_class("Yielder", object_class, ENUMERATOR_CLASS);
3439
unsafe { YIELDER_INIT.call_once(|| YIELDER = Some(yielder)) }
3540
globals.define_builtin_func(yielder.id(), "<<", yielder_push, 1);
3641
globals.define_builtin_func_rest(yielder.id(), "yield", yielder_yield);
@@ -461,6 +466,34 @@ mod tests {
461466
);
462467
}
463468

469+
#[test]
470+
fn yielder_yield_returns_nil() {
471+
// Issue #905: `Yielder#yield` must return nil (the value fed back on
472+
// resume), not the Yielder itself. Previously the generator driver
473+
// resumed the fiber with the yielder, so `y.yield(x)` returned the
474+
// Yielder; capturing it (`r << y.yield(1)`) leaked a Yielder into user
475+
// data, and inspecting it aborted the process (Array#inspect ->
476+
// Value::as_array on a non-array Yielder).
477+
run_test(
478+
r##"
479+
r = []
480+
Enumerator.new { |y| r << y.yield(1) }.to_a
481+
r
482+
"##,
483+
);
484+
run_test("Enumerator.new { |y| y.yield(1) }.first; :ok");
485+
// Yielder is an Object subclass (as in CRuby), and inspecting one no
486+
// longer crashes.
487+
run_test("Enumerator::Yielder.superclass.to_s");
488+
run_test(
489+
r##"
490+
y = nil
491+
Enumerator.new { |yy| y = yy; yy.yield 1 }.first
492+
[y.class.to_s, y.is_a?(Enumerator::Yielder)]
493+
"##,
494+
);
495+
}
496+
464497
#[test]
465498
fn enumerator_with_object() {
466499
run_tests(&[

monoruby/src/value/rvalue/fiber.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,15 @@ impl Fiber {
129129
yielder: Value,
130130
) -> Result<Value> {
131131
match self.state() {
132+
// First activation: the yielder is the block parameter (`|y|`).
132133
FiberState::Created => self.invoke_fiber(vm, globals, &[yielder]),
133-
FiberState::Suspended => self.resume_fiber(vm, yielder),
134+
// Subsequent resumes: this value becomes the return value of the
135+
// `y.yield(...)` that suspended the generator. For `each`/`to_a`
136+
// nothing is fed back, so it must be `nil` (CRuby's `Yielder#yield`
137+
// returns nil) — NOT the yielder itself, which previously leaked a
138+
// `Yielder` into user code (e.g. `r << y.yield(1)`) and then
139+
// aborted the process when that value was inspected.
140+
FiberState::Suspended => self.resume_fiber(vm, Value::nil()),
134141
FiberState::Terminated => Err(MonorubyErr::stopiterationerr(
135142
"iteration reached an end".to_string(),
136143
)),

spec/tags/core/enumerator/new_tags.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)