Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-48055] - Introduce convenient Channel retrieval methods in C…
…allables.

This change introduces new API, which is expected to cleanup FindBugs reports in the core and to offer better diagnostics of edge cases.
The logic also allows failing fast when the channel is being terminated.
  • Loading branch information
oleg-nenashev committed Nov 16, 2017
1 parent 40def98 commit df3dc72
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 6 deletions.
49 changes: 49 additions & 0 deletions src/main/java/hudson/remoting/Callable.java
Expand Up @@ -23,11 +23,18 @@
*/
package hudson.remoting;

import org.jenkinsci.remoting.ChannelStateException;
import org.jenkinsci.remoting.RoleSensitive;
import org.jenkinsci.remoting.SerializableOnlyOverRemoting;

import javax.annotation.Nonnull;
import java.io.IOException;
import java.io.NotSerializableException;
import java.io.Serializable;

//TODO: Make it SerializableOnlyOverRemoting?
// Oleg Nenashev: Formally there is no reason for that, you can serialize object over whatever binary stream and then
// execute it on remote side if it has channel instance. Likely YAGNI, but I can imagine such Pipeline context class implementation.
/**
* Represents computation to be done on a remote system.
*
Expand All @@ -40,4 +47,46 @@ public interface Callable<V,T extends Throwable> extends Serializable, RoleSensi
* or throws some exception.
*/
V call() throws T;

/**
* Gets a channel for the operation inside callable.
* @return Channel Instance
* @throws ChannelStateException Channel is not associated with the thread
* @since TODO
*/
@Nonnull
default Channel getChannelOrFail() throws ChannelStateException {
final Channel ch = Channel.current();
if (ch == null) {
// This logic does not prevent from improperly serializing objects within Remoting calls.
// If it happens in API calls in external usages, we wish good luck with diagnosing Remoting issues
// and leaks in ExportTable.
//TODO: maybe there is a way to actually diagnose this case?
final Thread t = Thread.currentThread();
throw new ChannelStateException("The calling thread " + t + " has no associated channel. "
+ "The current object " + this + " is " + SerializableOnlyOverRemoting.class +
", but it is likely being serialized/deserialized without the channel");
}
return ch;
}

/**
* Gets an open channel, which is ready to accept commands.
*
* It is a convenience method for cases, when a callable needs to invoke call backs on the master.
* In such case the requests will be likely failed by {@linkplain UserRequest} logic anyway, but it is better to fail fast.
*
* @return Channel instance
* @throws ChannelStateException The channel is closing down or has been closed.
* Also happens if the channel is not associated with the thread at all.
* @since TODO
*/
@Nonnull
default Channel getOpenChannelOrFail() throws ChannelStateException {
final Channel ch = getChannelOrFail();
if (ch.isClosingOrClosed()) {
throw new ChannelStateException("The associated channel " + ch + " is closing down or has closed down", ch.getCloseRequestCause());
}
return ch;
}
}
2 changes: 1 addition & 1 deletion src/main/java/hudson/remoting/RemoteInvocationHandler.java
Expand Up @@ -893,7 +893,7 @@ public RPCRequest(int oid, Method m, Object[] arguments, ClassLoader cl) {
}

public Serializable call() throws Throwable {
return perform(Channel.currentOrFail());
return perform(getChannelOrFail());
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/hudson/remoting/forward/PortForwarder.java
Expand Up @@ -134,9 +134,10 @@ public static ListeningPort create(VirtualChannel ch, final int acceptingPort, F

return ch.call(new Callable<ListeningPort,IOException>() {
public ListeningPort call() throws IOException {
final Channel channel = getOpenChannelOrFail(); // We initialize it early, so the forwarder won's start its daemon if the channel is shutting down
PortForwarder t = new PortForwarder(acceptingPort, proxy);
t.start();
return Channel.currentOrFail().export(ListeningPort.class,t);
return channel.export(ListeningPort.class,t);
}

@Override
Expand Down
47 changes: 47 additions & 0 deletions src/main/java/org/jenkinsci/remoting/ChannelStateException.java
@@ -0,0 +1,47 @@
/*
*
* The MIT License
*
* Copyright (c) 2017 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
*/

package org.jenkinsci.remoting;

import java.io.IOException;

//TODO: ClosedChannelException probably needs to be reworked in order to implement this type as well.
/**
* Indicates invalid state of the channel during the operation.
*
* @author Oleg Nenashev
* @since TODO
*/
public class ChannelStateException extends IOException {

public ChannelStateException(String message) {
super(message);
}

public ChannelStateException(String message, Throwable cause) {
super(message, cause);
}
}
2 changes: 1 addition & 1 deletion src/test/java/hudson/remoting/ChannelTest.java
Expand Up @@ -130,7 +130,7 @@ public void testWaitForRemoteProperty() throws Exception {
private static class WaitForRemotePropertyCallable extends CallableBase<Void, Exception> {
public Void call() throws Exception {
Thread.sleep(500);
Channel.currentOrFail().setProperty("foo","bar");
getChannelOrFail().setProperty("foo","bar");
return null;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/hudson/remoting/ClassRemotingTest.java
Expand Up @@ -204,7 +204,7 @@ public static Test suite() throws Exception {

private static class RemotePropertyVerifier extends CallableBase<Object, IOException> {
public Object call() throws IOException {
Object o = Channel.currentOrFail().getRemoteProperty("test");
Object o = getOpenChannelOrFail().getRemoteProperty("test");
assertEquals(o.getClass().getName(), CLASSNAME);
assertTrue(Channel.class.getClassLoader() != o.getClass().getClassLoader());
assertTrue(o.getClass().getClassLoader() instanceof RemoteClassLoader);
Expand Down
Expand Up @@ -1175,7 +1175,7 @@ private static class ProbeCallable implements Callable<String, IOException> {

@Override
public String call() throws IOException {
System.out.println("Hello from: " + Channel.currentOrFail());
System.out.println("Hello from: " + getChannelOrFail());
return null;
}

Expand All @@ -1193,7 +1193,7 @@ public CreateSaturationTestProxy(hudson.remoting.Pipe pipe) {
}

public ISaturationTest call() throws IOException {
return Channel.currentOrFail().export(ISaturationTest.class, new ISaturationTest() {
return getOpenChannelOrFail().export(ISaturationTest.class, new ISaturationTest() {
private InputStream in;

public void ensureConnected() throws IOException {
Expand Down

0 comments on commit df3dc72

Please sign in to comment.