From c8b08d9e12182dca6ad0793146d18e2e7315e66e Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Tue, 3 Dec 2024 11:05:33 -0600 Subject: [PATCH] Avoid re-polling for events when printing error Dynamically printing error output to the configured stderr stream may trigger additional thread interrupt polls, wiping out any thread interrupt currently being reported or propagated. This patch avoids such polls by writing directly to the underlying IO without using Ruby logic if and only if it is the original (boot) stderr stream. Fixes jruby/jruby#8479 --- core/src/main/java/org/jruby/Ruby.java | 95 +++++++++++++++++--- core/src/main/java/org/jruby/RubyGlobal.java | 2 + core/src/main/java/org/jruby/RubyThread.java | 15 +++- 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java index a7d2ab1bc26..200dea42fe4 100644 --- a/core/src/main/java/org/jruby/Ruby.java +++ b/core/src/main/java/org/jruby/Ruby.java @@ -172,10 +172,15 @@ import java.io.InputStream; import java.io.PrintStream; import java.io.PrintWriter; +import java.io.Writer; import java.lang.invoke.MethodHandle; import java.lang.ref.WeakReference; import java.net.BindException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.WritableByteChannel; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.security.SecureRandom; import java.util.ArrayList; @@ -2848,6 +2853,25 @@ WarnCallback getRegexpWarnings() { return regexpWarnings; } + public IRubyObject getStderr() { + return getGlobalVariables().get("$stderr"); + } + + /** + * Return the original stderr with which this runtime was initialized. + * + * Used for fast-path comparisons when printing error info directly to stderr. + * + * @return the original stderr with which this runtime was initialized + */ + public IRubyObject getOriginalStderr() { + return originalStderr; + } + + void setOriginalStderr(IRubyObject stderr) { + this.originalStderr = stderr; + } + public PrintStream getErrorStream() { // FIXME: We can't guarantee this will always be a RubyIO...so the old code here is not safe /*java.io.OutputStream os = ((RubyIO) getGlobalVariables().getService("$stderr")).getOutStream(); @@ -2916,7 +2940,8 @@ private static boolean isJavaPackageOrJavaClassProxyType(final RubyModule type) return type instanceof JavaPackage || ClassUtils.isJavaClassProxyType(type); } - /** Prints an error with backtrace to the error stream. + /** + * Prints a Ruby exception with backtrace to the configured stderr stream. * * MRI: eval.c - error_print() * @@ -2924,15 +2949,20 @@ private static boolean isJavaPackageOrJavaClassProxyType(final RubyModule type) public void printError(final RubyException ex) { if (ex == null) return; - PrintStream errorStream = getErrorStream(); - String backtrace = config.getTraceType().printBacktrace(ex, (errorStream == System.err) && getPosix().isatty(FileDescriptor.err)); - try { - errorStream.print(backtrace); - } catch (Exception e) { - System.err.print(backtrace); - } + boolean formatted = + getStderr() == getOriginalStderr() && + getErr() == System.err && + getPosix().isatty(FileDescriptor.err); + + String backtrace = config.getTraceType().printBacktrace(ex, formatted); + printErrorString(backtrace); } + /** + * Prints an exception to System.err. + * + * @param ex + */ public void printError(final Throwable ex) { if (ex instanceof RaiseException) { printError(((RaiseException) ex).getException()); @@ -2940,12 +2970,11 @@ public void printError(final Throwable ex) { } ByteArrayOutputStream baos = new ByteArrayOutputStream(); - PrintStream errorStream = getErrorStream(); ex.printStackTrace(new PrintStream(baos)); try { - errorStream.write(baos.toByteArray()); + printErrorString(baos.toByteArray()); } catch (Exception e) { try { System.err.write(baos.toByteArray()); @@ -2956,6 +2985,50 @@ public void printError(final Throwable ex) { } } + /** + * Prints a string directly to the stderr channel, if default, or via dynamic dispatch otherwise. + * + * @param msg the string to print + */ + public void printErrorString(String msg) { + IRubyObject stderr = getStderr(); + + WritableByteChannel writeChannel; + if (stderr == getOriginalStderr() && + (writeChannel = ((RubyIO) stderr).getOpenFile().fd().chWrite) != null) { + Writer writer = Channels.newWriter(writeChannel, "UTF-8"); + try { + writer.write(msg); + writer.flush(); + } catch (IOException ioe) { + // ignore as in CRuby + } + } else { + getErrorStream().print(msg); + } + } + + /** + * Prints a string directly to the stderr channel, if default, or via dynamic dispatch otherwise. + * + * @param msg the string to print + */ + public void printErrorString(byte[] msg) { + IRubyObject stderr = getGlobalVariables().get("$stderr"); + + try { + WritableByteChannel writeChannel; + if (stderr == getOriginalStderr() && + (writeChannel = ((RubyIO) stderr).getOpenFile().fd().chWrite) != null) { + writeChannel.write(ByteBuffer.wrap(msg)); + } else { + getErrorStream().write(msg); + } + } catch (IOException ioe) { + // ignore as in CRuby + } + } + static final String ROOT_FRAME_NAME = "(root)"; static long yarpTime = 0; static boolean loaded = false; @@ -5724,6 +5797,8 @@ public void warn(String message) { private final EnumMap definedMessages = new EnumMap<>(DefinedMessage.class); private final EnumMap threadStatuses = new EnumMap<>(RubyThread.Status.class); + private IRubyObject originalStderr; + public interface ObjectSpacer { void addToObjectSpace(Ruby runtime, boolean useObjectSpace, IRubyObject object); } diff --git a/core/src/main/java/org/jruby/RubyGlobal.java b/core/src/main/java/org/jruby/RubyGlobal.java index 663d47e4cde..75649832b04 100644 --- a/core/src/main/java/org/jruby/RubyGlobal.java +++ b/core/src/main/java/org/jruby/RubyGlobal.java @@ -311,6 +311,8 @@ public static void initSTDIO(Ruby runtime, GlobalVariables globals) { runtime.defineGlobalConstant("STDIN", stdin); runtime.defineGlobalConstant("STDOUT", stdout); runtime.defineGlobalConstant("STDERR", stderr); + + runtime.setOriginalStderr(stderr); } else { ((RubyIO) runtime.getObject().getConstant("STDIN")).getOpenFile().setFD(stdin.getOpenFile().fd()); ((RubyIO) runtime.getObject().getConstant("STDOUT")).getOpenFile().setFD(stdout.getOpenFile().fd()); diff --git a/core/src/main/java/org/jruby/RubyThread.java b/core/src/main/java/org/jruby/RubyThread.java index 5c5fe9f69f6..50fc849f533 100644 --- a/core/src/main/java/org/jruby/RubyThread.java +++ b/core/src/main/java/org/jruby/RubyThread.java @@ -34,13 +34,17 @@ package org.jruby; import java.io.IOException; +import java.io.Writer; import java.lang.management.ManagementFactory; import java.lang.ref.WeakReference; import java.nio.ByteBuffer; import java.nio.channels.Channel; +import java.nio.channels.Channels; import java.nio.channels.SelectableChannel; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; +import java.nio.channels.WritableByteChannel; +import java.nio.charset.StandardCharsets; import java.util.Iterator; import java.util.Objects; import java.util.Queue; @@ -275,7 +279,6 @@ private void executeInterrupts(ThreadContext context, boolean blockingTiming) { ((RubyFixnum) err).getLongValue() == 2)) { toKill(); } else { - afterBlockingCall(); if (getStatus() == Status.SLEEP) { exitSleep(); } @@ -1769,12 +1772,14 @@ private Return executeTask(ThreadContext context, Data data, Stat STATUS.set(this, status); - return task.run(context, data); + Return ret = task.run(context, data); + + return ret; } finally { STATUS.set(this, oldStatus); this.unblockFunc = null; this.unblockArg = null; - pollThreadEvents(context, blocking); + pollThreadEvents(context); } } @@ -2071,7 +2076,9 @@ public void exceptionRaised(RaiseException exception) { protected void printReportExceptionWarning() { Ruby runtime = getRuntime(); String name = threadImpl.getReportName(); - runtime.getErrorStream().println("warning: thread \"" + name + "\" terminated with exception (report_on_exception is true):"); + String warning = "warning: thread \"" + name + "\" terminated with exception (report_on_exception is true):"; + + runtime.printErrorString(warning); } /**