[PATCH] Fix JMC-5398

[Marcus Hirt]

Hi Joshua,

Greetings from JCrete! Looks good! I realized that there are a few NON-NLS tags missing in core elsewhere. Will open a new bug and simply just push the changes (trivial), unless someone disagrees.

Kind regards,
Marcus

On 2018-07-20, 18:07, "jmc-dev on behalf of Mario Torre" <jmc-dev-bounces at openjdk.java.net on behalf of neugens at redhat.com> wrote:

    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