Skip to content

Commit

Permalink
Don't use "ready to throw" utility methods
Browse files Browse the repository at this point in the history
The Ruby.newBlahError methods are expected to return a fully-
prepared RaiseException with stack trace and current $! set. If
these methods are used to create a RubyException for throwing on
another thread, the current thread's stack trace and $! will be
used incorrectly.

Instead, I start the process of adding direct constructors for
Ruby exceptions, starting with RubyIOError.newIOError, which will
return just the Ruby exception object without any preparation for
throwing.
  • Loading branch information
headius committed Nov 6, 2023
1 parent 5428a72 commit 01e585c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
4 changes: 4 additions & 0 deletions core/src/main/java/org/jruby/RubyIOError.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,8 @@ static RubyClass define(Ruby runtime, RubyClass exceptionClass) {
protected RaiseException constructThrowable(String message) {
return new IOError(message, this);
}

public static RubyIOError newIOError(Ruby runtime, String message) {
return ((RubyIOError) runtime.getIOError().newInstance(runtime.getCurrentContext(), RubyString.newString(runtime, message)));
}
}
9 changes: 5 additions & 4 deletions core/src/main/java/org/jruby/util/io/OpenFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.jruby.RubyException;
import org.jruby.RubyFixnum;
import org.jruby.RubyIO;
import org.jruby.RubyIOError;
import org.jruby.RubyNumeric;
import org.jruby.FiberScheduler;
import org.jruby.RubyString;
Expand Down Expand Up @@ -1353,7 +1354,7 @@ private ChannelFD preRead(ThreadContext context, OpenFile fptr) {

// We should not get here without previously checking, so this must have been closed by another thread
if (fd == null) {
throw newInterruptedException(context.runtime);
throw newInterruptedException(context.runtime).toThrowable();
}

assert fptr.lockedByMe();
Expand Down Expand Up @@ -2904,14 +2905,14 @@ public void interruptBlockingThreads(ThreadContext context) {
if (thread == context.getThread()) continue;

// raise will also wake the thread from selection
RubyException exception = newInterruptedException(runtime).getException();
RubyException exception = newInterruptedException(runtime);
thread.raise(exception);
}
}
}

static RaiseException newInterruptedException(Ruby runtime) {
return runtime.newIOError("stream closed in another thread");
static RubyIOError newInterruptedException(Ruby runtime) {
return RubyIOError.newIOError(runtime, "stream closed in another thread");
}

/**
Expand Down

0 comments on commit 01e585c

Please sign in to comment.