Skip to content
Permalink
Browse files

[JENKINS-37566] - Suppress issue with non-serializable inner classes …

…in NioChannelHub (#231)

* [JENKINS-37566] - Supress issue with non-serializable inner classes in NioChannelHub.

The wrapper may decrease the performance a bit, but NioChannelHub is not being used in JNLP4 anyway.
Ideally submissions should not use the selector tasks queue in such lame way, but it’s risky to change it to another executor service.

* [JENKINS-37566] - Address comments from @stephenc and @rysteboe

* [JENKINS-37566] Do not even try to serialize CallableRemotingWrapper as @stephenc suggests
  • Loading branch information...
oleg-nenashev committed Nov 29, 2017
1 parent 5412f28 commit c186279ee8a671c09afb322d2f75ec9166e64fa3
Showing with 55 additions and 22 deletions.
  1. +55 −22 src/main/java/org/jenkinsci/remoting/nio/NioChannelHub.java
@@ -19,6 +19,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.InterruptedIOException;
import java.io.NotSerializableException;
import java.io.ObjectStreamException;
import java.io.OutputStream;
import java.nio.channels.CancelledKeyException;
import java.nio.channels.ClosedSelectorException;
@@ -39,6 +41,7 @@
import static java.nio.channels.SelectionKey.*;
import static java.util.logging.Level.*;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* Switch board of multiple {@link Channel}s through NIO select.
@@ -253,33 +256,19 @@ public void closeWrite() throws IOException {

@Override
public void closeRead() throws IOException {
scheduleSelectorTask(new Callable<Void, IOException>() {
public Void call() throws IOException {
closeR();
return null;
}

@Override
public void checkRoles(RoleChecker checker) throws SecurityException {
throw new AssertionError(); // not actually used over remoting
}
scheduleSelectorTask(() -> {
closeR();
return null;
});
}

/**
* Update the operations for which we are registered.
*/
private void scheduleReregister() {
scheduleSelectorTask(new Callable<Void, IOException>() {
public Void call() throws IOException {
reregister();
return null;
}

@Override
public void checkRoles(RoleChecker checker) throws SecurityException {
throw new AssertionError(); // not actually used over remoting
}
scheduleSelectorTask(() -> {
reregister();
return null;
});
}

@@ -508,11 +497,55 @@ protected CommandTransport makeTransport(InputStream is, OutputStream os, Mode m
};
}

private void scheduleSelectorTask(Callable<Void, IOException> task) {
selectorTasks.add(task);
// TODO: This logic should use Executor service
private void scheduleSelectorTask(java.util.concurrent.Callable<Void> task) {
selectorTasks.add(new CallableRemotingWrapper(task));
selector.wakeup();
}

/**
* Provides a wrapper for submitting {@link java.util.concurrent.Callable}s over Remoting execution queues.
*
* @deprecated It is just a hack, which schedules non-serializable tasks over the Remoting Task queue.
* There is no sane reason to reuse this wrapper class anywhere.
*/
@Deprecated
private static final class CallableRemotingWrapper implements Callable<Void, IOException> {
private static final long serialVersionUID = -7331104479109353930L;
final transient java.util.concurrent.Callable<Void> task;

CallableRemotingWrapper(@Nonnull java.util.concurrent.Callable<Void> task) {
this.task = task;
}

@Override
public Void call() throws IOException {
if (task == null) {
throw new IOException("The callable " + this + " has been serialized somehow, but it is actually not serializable");
}
try {
return task.call();
} catch (IOException ex) {
throw ex;
} catch (Exception ex) {
throw new IOException(ex);
}
}

private Object readResolve() throws ObjectStreamException {
throw new NotSerializableException("The class should not be serialized over Remoting");
}

private Object writeReplace() throws ObjectStreamException {
throw new NotSerializableException("The class should not be serialized over Remoting");
}

@Override
public void checkRoles(RoleChecker checker) throws SecurityException {
throw new SecurityException("The class should not be serialized over Remoting");
}
}

/**
* Shuts down the selector thread and aborts all
*/

0 comments on commit c186279

Please sign in to comment.
You can’t perform that action at this time.