[Bug 62148] New: NullPointerException in PumpStreamHandler

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62148

            Bug ID: 62148
           Summary: NullPointerException in PumpStreamHandler
           Product: Ant
           Version: nightly
          Hardware: Macintosh
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Core
          Assignee: notifications@ant.apache.org
          Reporter: mjkim0324@hotmail.com
  Target Milestone: ---

Hello.

I have a unit test for PumpStreamHandler that throws NullPointerException as
follows.

public void test() {
  PumpStreamHandler pump = new PumpStreamHandler();
  pump.start();
}

public void start() {
    outputThread.start(); //NPE occurs
    errorThread.start();
    if (inputThread != null) {
        inputThread.start();
    }
}

I'm wondering why outputThread and errorThread are not null-checked like
inputThread. Is this intentional or a bug?

Thank you for your time.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62148] NullPointerException in PumpStreamHandler

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62148

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |1.10.4
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #2 from Stefan Bodewig <bo...@apache.org> ---
I just now realized I misread your bug report. The err/out streams must not be
null and I've changed the code to actually verify that.

For the threads themselves it probably is a good idea to null-check them, even
though they will never be null in any realistic setting.

Both changes are inside the 1.9.x and master branches now.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 62148] NullPointerException in PumpStreamHandler

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62148

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from Stefan Bodewig <bo...@apache.org> ---
If you look through Ant's code, you will see PumpStreamHandler is only ever
used from within Execute (apart from invoking the constructor which happens in
a few different places) and inside of this context we "know" outputThread can
never be null. All three setProcess*Stream methods will have been called in
either case before start is called.

Actually technically there may even be a bug that errorThread could be null,
but this doesn't seem to happen in practice or we would have bug reports. The
"err != null" check in setProcessErrorStream looks wrong, neither out nor err
are allowed to be null or we'd end up with NPEs in stop - and in the case of
err == null would  not consume a process' error stream which may block the
process eventually.

The class is not designed to be used outside of the context if Execute at all.
It is public for technical reasons but not expected to be used as an public
API. I hope you are writing a test because you "just" want to write a test and
not because you want to re-use the class outside of Ant.

That being said it certainly wouldn't hurt if the code was more defensive and
verified the implicit preconditions.

-- 
You are receiving this mail because:
You are the assignee for the bug.