[PATCH] Fix JMC-5398

[Marcus Hirt]

Hi Joshua,

 

Looks fine! Mario should be able to sponsor your patch – he is a committer.

 

Kind regards,

Marcus

 

From: Joshua Matsuoka <jmatsuok at redhat.com>
Date: Tuesday, 24 July 2018 at 17:20
To: Marcus Hirt <marcus.hirt at oracle.com>
Cc: Mario Torre <neugens at redhat.com>, <jmc-dev at openjdk.java.net>
Subject: Re: [PATCH] Fix JMC-5398

 

Hi Marcus, Mario,

 

Here's the updated patch with the missing NON-NLS-1 tag added.

 

Cheers,

 

- Josh

 

On Tue, Jul 24, 2018 at 10:58 AM, Marcus Hirt <marcus.hirt at oracle.com> wrote:

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