RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

[JC Beyler]

So that's what I thought too but I did this:

$ cat Test.java
public class Test {
  public static void main(String[] args) {
    int res;

    try {
      res = 5;
    } catch (Exception e) {
    }

    if (res > 0) {
    }
  }
}

That fails to compile with JDK8 and JDK11 it seems. Perhaps I'm doing
something wrong?

$ javac Test.java
Test.java:10: error: variable res might not have been initialized
    if (res > 0) {
        ^
1 error


Sorry if I'm derailing the review :)

Jc


On Wed, Oct 10, 2018 at 7:36 PM David Holmes <david.holmes at oracle.com>
wrote:

> On 11/10/2018 12:19 PM, JC Beyler wrote:
> > Hi Alex,
> >
> > -
> >
> http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html
> > <
> http://cr.openjdk.java.net/%7Eamenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html
> >
> >    -> Why not make it javadoc like the other methods of the same file
> > (so @return instead of returns and a second * at the start of the
> comment)?
> >
> > -
> >
> http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/jdk/com/sun/jdi/BasicJDWPConnectionTest.java.udiff.html
> > <
> http://cr.openjdk.java.net/%7Eamenkov/BasicJDWPConn/webrev.01/test/jdk/com/sun/jdi/BasicJDWPConnectionTest.java.udiff.html
> >
> > a)   I'm surprised by this:
> >
> > + int res;
> > + try {
> > + res = handshake(detectPort(a.getProcessStdout()));
> > + } finally {
> >           a.stopApp();
> > + }
> >           if (res < 0) {
> >
> >
> > I would have thought that this makes javac return a "res might not be
> initialized" error.
>
> res can only be uninitialized if an exception occurs, in which case you
> won't reach the "if (res , 0)" statement.
>
> David
> -----
>
> >
> > b) Nit: Is there a reason we are complicating the code here:
> >
> >           try {
> >               LingeredApp a = LingeredApp.startApp(cmd);
> > +
> > + // startApp is expected to fail, but if not, terminate the app
> > + try {
> > + a.stopApp();
> > + } catch (IOException e) {
> > + // print and let the test fail
> > + System.err.println("LingeredApp.stopApp failed");
> > + e.printStackTrace();
> > + }
> >           } catch (IOException ex) {
> >               System.err.println(testName + ": caught expected
> IOException");
> >               System.err.println(testName + " PASSED");
> >               return;
> >           }
> >
> > Why not just put it below? We could either put a outside the try and
> then move that code out; or perhaps move it into a separate method to let
> >
> > the reader concentrate on the test at hand and let the "stopping of the
> app"  happen somewhere else?
> >
> >
> > Thanks,
> > Jc
> >
> > On Wed, Oct 10, 2018 at 5:18 PM serguei.spitsyn at oracle.com
> > <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com
> > <mailto:serguei.spitsyn at oracle.com>> wrote:
> >
> >     Hi Alex,
> >
> >     It looks good to me.
> >     How did you test it?
> >
> >     Thanks,
> >     Serguei
> >
> >
> >     On 10/10/18 16:25, Alex Menkov wrote:
> >      > Hi all,
> >      >
> >      > please review a fix for
> >      > https://bugs.openjdk.java.net/browse/JDK-8195703
> >      > webrev:
> >      > http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/
> >     <http://cr.openjdk.java.net/%7Eamenkov/BasicJDWPConn/webrev.01/>
> >      >
> >      > I was not able to reproduce the issue, but accordingly the logs in
> >      > jira root cause is a transport initialization error "Address
> already
> >      > in use".
> >      > The test uses Utils.getFreePort() to select some free port, but
> >     it can
> >      > be race condition when some other app (or other test) uses the
> port
> >      > selected before debuggee application starts to listen on it.
> >      > The fix uses dynamic port allocation and then parses it from the
> >      > debuggee output.
> >      > Other changes:
> >      > - dropped catching exceptions and calling System.exit() - this
> >     causes
> >      > SecurityException in JTReg harness which makes error analysis much
> >      > harder;
> >      > - dropped using of Utils.getFreePort() from
> jdi/DoubleAgentTest.java
> >      > test;
> >      >   jdi/BadHandshakeTest.java also uses Utils.getFreePort(), but it
> >      > handles "Already in use" error re-peeking other free port and
> >      > restarting debuggee, so I keep it as is.
> >      >
> >      > --alex
> >      >
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181010/9b8fca35/attachment-0001.html>