Skip to content

Commit

Permalink
Avoid re-polling for events when printing error
Browse files Browse the repository at this point in the history
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#8479
  • Loading branch information
headius committed Dec 3, 2024
1 parent eee97c9 commit c8b08d9
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 14 deletions.
95 changes: 85 additions & 10 deletions core/src/main/java/org/jruby/Ruby.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -2916,36 +2940,41 @@ 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()
*
*/
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());
return;
}

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());
Expand All @@ -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;
Expand Down Expand Up @@ -5724,6 +5797,8 @@ public void warn(String message) {
private final EnumMap<DefinedMessage, RubyString> definedMessages = new EnumMap<>(DefinedMessage.class);
private final EnumMap<RubyThread.Status, RubyString> threadStatuses = new EnumMap<>(RubyThread.Status.class);

private IRubyObject originalStderr;

public interface ObjectSpacer {
void addToObjectSpace(Ruby runtime, boolean useObjectSpace, IRubyObject object);
}
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/org/jruby/RubyGlobal.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
15 changes: 11 additions & 4 deletions core/src/main/java/org/jruby/RubyThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -275,7 +279,6 @@ private void executeInterrupts(ThreadContext context, boolean blockingTiming) {
((RubyFixnum) err).getLongValue() == 2)) {
toKill();
} else {
afterBlockingCall();
if (getStatus() == Status.SLEEP) {
exitSleep();
}
Expand Down Expand Up @@ -1769,12 +1772,14 @@ private <Data, Return> 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);
}
}

Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit c8b08d9

Please sign in to comment.