Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-39150] - API stabilization && compliance with the compatibility policy #125

6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ THE SOFTWARE.
<version>3.0.1</version>
<type>jar</type>
</dependency>
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<version>1.7</version>
<type>jar</type>
</dependency>
</dependencies>

<build>
Expand Down
35 changes: 29 additions & 6 deletions src/main/java/hudson/remoting/Channel.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Represents a communication channel to the remote peer.
Expand Down Expand Up @@ -1172,6 +1174,7 @@ public void dumpPerformanceCounters(PrintWriter w) throws IOException {
w.printf(Locale.ENGLISH, "Resource loading time=%,dms%n", resourceLoadingTime.get() / (1000 * 1000));
}

//TODO: Make public after merge into the master branch
/**
* Print the diagnostic information.
*
Expand Down Expand Up @@ -1226,9 +1229,14 @@ public void dumpPerformanceCounters(PrintWriter w) throws IOException {
* Number of RPC calls (e.g., method call through a {@linkplain RemoteInvocationHandler proxy})
* that the other side has made to us but not yet returned yet.
*
* @since 2.26.3
* @param w Output destination
* @throws IOException Error while creating or writing the channel information
*
* @since 2.62.3 - stable 2.x (restricted)
* @since TODO 3.x - public version
*/
public void dumpDiagnostics(PrintWriter w) throws IOException {
@Restricted(NoExternalUse.class)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not effectively prevent from usage in libraries, which do not invoke accmod checks in the build process. Is it fine enough?

public void dumpDiagnostics(@Nonnull PrintWriter w) throws IOException {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe IOException is YAGNI, but I would like to keep it since one may override Channel implementation and start throwing something. The current implementation does not throw exceptions

w.printf("Channel %s%n",name);
w.printf(" Created=%s%n", new Date(createdAt));
w.printf(" Commands sent=%d%n", commandsSent);
Expand Down Expand Up @@ -1584,17 +1592,32 @@ public static Channel current() {
return CURRENT.get();
}

// TODO: Unrestrict after the merge into the master.
// By now one may use it via the reflection logic only
/**
* Calls {@link #dumpDiagnostics(PrintWriter)} across all the active channels in this system.
* Used for diagnostics.
*
* @since 2.26.3
* @param w Output destination
*
* @since 2.62.3 - stable 2.x (restricted)
* @since TODO 3.x - public version
*/
public static void dumpDiagnosticsForAll(PrintWriter w) throws IOException {
@Restricted(NoExternalUse.class)
public static void dumpDiagnosticsForAll(@Nonnull PrintWriter w) {
for (Ref ref : ACTIVE_CHANNELS.values().toArray(new Ref[0])) {
Channel ch = ref.channel();
if (ch!=null)
ch.dumpDiagnostics(w);
if (ch != null) {
try {
ch.dumpDiagnostics(w);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are performance concerned, I would recommend adding

if (w.checkError()) {
  return; 
}

as no sense continuing if PrintWriter has already suppressed the IOE

} catch (Throwable ex) {
if (ex instanceof Error) {
throw (Error)ex;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily helpful. (For ThreadDeath, yes; for LinkageError, generally no; for OutOfMemoryError, probably irrelevant.)

}
logger.log(Level.WARNING,
String.format("Cannot dump diagnostics for the channel %s", ch.getName()), ex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably send this to the print writer too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

}
}
}
}

Expand Down