[PATCH] Fix JMC-5398

[Mario Torre]

Hi Joshua,

The patch looks good, however I would like a second reviewer,
especially considering that someone should push your patch if accepted
(which I can do of course but at this stage I still prefer if someone
from Oracle does it/or gives the OK).

I have only one nitpick:

+    private static final String ATTACH_TIMED_OUT_ERROR_MESSAGE =
"Timed out attempting to attach to target JVM!";

I believe that if this is not marked for translation it should be
followed by //$NON-NLS-1$ or eclipse will mark a warning about this
line.

I'm not sure if this code will work on Java 9+, of course that's a
concern valid for the existing code too, I'm not sure if Marcus prefer
to address that in one go or with a separate patch (assuming there may
be other areas of interest, perhaps a separate patch is better, I
didn't see a bug regarding this, perhaps we should file one?).

Cheers,
Mario


On Thu, Jul 19, 2018 at 11:39 PM, Joshua Matsuoka <jmatsuok at redhat.com> wrote:
> Hi,
>
> The following patch fixes JMC-5398 [1]. The bug was twofold, first when
> attempting to attach to a JVM that was either busy or suspended JMC would
> hang, and second upon restarting the JVM browser would not list any JVMs.
>
> This patch adds a timeout to the problematic calls. This ensures that
> during JVM discovery problematic JVMs are skipped when attaching times out,
> keeping the JVM browser functioning properly, as well as ensuring that we
> don't hang when attempting to open the JMX console or start a flight
> recording for a problematic JVM.
>
> The timeout is currently 5 seconds but can easily be changed if this is too
> short/long.
>
> [1]
> https://bugs.openjdk.java.net/projects/JMC/issues/JMC-5398?filter=allopenissues
>
> Thoughts?
>
> Cheers,
>
> - Josh



-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898