Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport 3.3: YJIT memory leak fix with additional CI fixes #9841

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Feb 5, 2024

This is a backport PR for https://bugs.ruby-lang.org/issues/20209
When we landed the singleton class fix on master, it revealed some unrelated panics with tailcalls which destabilized CI. So I'm making a PR to group everything together to keep the backport branch green.


YJIT: Avoid leaks by skipping objects with a singleton class

For receiver with a singleton class, there are multiple vectors YJIT can
end up retaining the object. There is a path in jit_guard_known_klass()
that bakes the receiver into the code, and the object could also be kept
alive indirectly through a path starting at the CME object baked into
the code.

To avoid these leaks, avoid compiling calls on objects with a singleton
class.

See: https://github.com/Shopify/ruby/issues/552

[Bug #20209]
---
 yjit/bindgen/src/main.rs       |  1 +
 yjit/src/codegen.rs            | 17 +++++++++++++++++
 yjit/src/cruby_bindings.inc.rs |  1 +
 yjit/src/stats.rs              |  2 ++
 4 files changed, 21 insertions(+)

YJIT: Fix tailcall and JIT entry eating up FINISH frames (#9729)

Suppose YJIT runs a rb_vm_opt_send_without_block()
fallback and the control frame stack looks like:

```
will_tailcall_bar [FINISH]
caller_that_used_fallback
```

will_tailcall_bar() runs in the interpreter and sets up a tailcall.
Right before JIT_EXEC() in the `send` instruction, the stack will look like:

```
bar [FINISH]
caller_that_used_fallback
```

Previously, JIT_EXEC() ran bar() in JIT code, which caused the `FINISH`
flag to return to the interpreter instead of to the JIT code running
caller_that_used_fallback(), causing code to run twice and probably
crash. Recent flaky failures on CI about "each stub expects a particular
iseq" are probably due to leaving methods twice in
`test_optimizations.rb`.

Only run JIT code from the interpreter if a new frame is pushed.
---
 test/ruby/test_optimization.rb | 11 +++++++++++
 vm_exec.h                      |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

YJIT: No need to RESTORE_REG now that we reject tailcalls

Thanks to Kokubun for noticing.

Follow-up: b0711b1cf152afad0a480ee2f9bedd142a0d24ac
---
 vm_exec.h | 1 -
 1 file changed, 1 deletion(-)

	YJIT: Avoid leaks by skipping objects with a singleton class

	For receiver with a singleton class, there are multiple vectors YJIT can
	end up retaining the object. There is a path in jit_guard_known_klass()
	that bakes the receiver into the code, and the object could also be kept
	alive indirectly through a path starting at the CME object baked into
	the code.

	To avoid these leaks, avoid compiling calls on objects with a singleton
	class.

	See: Shopify#552

	[Bug #20209]
	---
	 yjit/bindgen/src/main.rs       |  1 +
	 yjit/src/codegen.rs            | 17 +++++++++++++++++
	 yjit/src/cruby_bindings.inc.rs |  1 +
	 yjit/src/stats.rs              |  2 ++
	 4 files changed, 21 insertions(+)

	YJIT: Fix tailcall and JIT entry eating up FINISH frames (ruby#9729)

	Suppose YJIT runs a rb_vm_opt_send_without_block()
	fallback and the control frame stack looks like:

	```
	will_tailcall_bar [FINISH]
	caller_that_used_fallback
	```

	will_tailcall_bar() runs in the interpreter and sets up a tailcall.
	Right before JIT_EXEC() in the `send` instruction, the stack will look like:

	```
	bar [FINISH]
	caller_that_used_fallback
	```

	Previously, JIT_EXEC() ran bar() in JIT code, which caused the `FINISH`
	flag to return to the interpreter instead of to the JIT code running
	caller_that_used_fallback(), causing code to run twice and probably
	crash. Recent flaky failures on CI about "each stub expects a particular
	iseq" are probably due to leaving methods twice in
	`test_optimizations.rb`.

	Only run JIT code from the interpreter if a new frame is pushed.
	---
	 test/ruby/test_optimization.rb | 11 +++++++++++
	 vm_exec.h                      |  3 ++-
	 2 files changed, 13 insertions(+), 1 deletion(-)

	YJIT: No need to RESTORE_REG now that we reject tailcalls

	Thanks to Kokubun for noticing.

	Follow-up: b0711b1
	---
	 vm_exec.h | 1 -
	 1 file changed, 1 deletion(-)
@XrXr XrXr added the Backport label Feb 5, 2024
@XrXr XrXr changed the title Backport 3.3: YJIT memory leak fix with additional fixes Backport 3.3: YJIT memory leak fix with additional CI fixes Feb 5, 2024
@nurse nurse added this pull request to the merge queue Mar 14, 2024
Merged via the queue into ruby:ruby_3_3 with commit cdcabd8 Mar 14, 2024
98 checks passed
@XrXr XrXr deleted the yjit-singleton-class-backport branch March 14, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants