On 4 sep 2014, at 14:09, Dmitry Samersoff <[email protected]> wrote:
> Staffan, > > On 2014-09-04 16:02, Staffan Larsen wrote: >> >> On 4 sep 2014, at 13:52, Dmitry Samersoff <[email protected]> >> wrote: >> >>> Staffan, >>> >>> WindowsVirtualMachine.java:109 Should we close PipedInputStream ? >> >> There is a catch-statement that does this for IOExceptions on line 125. >> Since AttachOperationFailedException is an IOException it the pipe will get >> closed there. It really should be a finally (or try-with-resources) block >> instead. > > At ll. 125 we are closing pipe: > > closePipe(hPipe); > > But It might be necessary to close PipedInputStream as well. Ah, right - I misunderstood what you meant. Yes, the PipedInputStream should be closed before we throw an exception. Since this is not something I introduced with this change I have filed a bug for it and will deal with it separately: https://bugs.openjdk.java.net/browse/JDK-8057558 Thanks, /Staffan > > -Dmitry > > >> >> Thanks, >> /Staffan >> >>> >>> Otherwise looks good. >>> >>> -Dmitry >>> >>> On 2014-09-04 14:11, Staffan Larsen wrote: >>>> Can I interest any Reviewers in taking a look at this? >>>> >>>> Thanks, >>>> /Staffan >>>> >>>> On 3 sep 2014, at 14:27, Staffan Larsen <[email protected]> wrote: >>>> >>>>> …aaaand the link: >>>>> http://cr.openjdk.java.net/~sla/8044398-8039173-8044135-jdk8u/webrev.00/ >>>>> >>>>> /Staffan >>>>> >>>>> On 3 sep 2014, at 14:25, Staffan Larsen <[email protected]> wrote: >>>>> >>>>>> This is a review for a backport of JDK-8044135 to jdk8u. This fix had a >>>>>> few dependencies that also needed backporting, so included in this >>>>>> review are the following fixes from JDK9: >>>>>> >>>>>> • 8044135 Add API to start JMX agent from attach framework (jdk) >>>>>> • 8039173 Propagate errors from Diagnostic Commands as exceptions in the >>>>>> attach framework (jdk) >>>>>> • 8044398 Attach code should propagate errors in Diagnostic Commands as >>>>>> errors (hotspot) >>>>>> >>>>>> The only significant change from the version in JDK 9 is that >>>>>> startManagementAgent() and startLocalManagementAgent() in >>>>>> VirtualMachine.java have "@since 1.8" (instead of 1.9). >>>>>> >>>>>> After speaking with the maintainers and gatekeepers, I plan to push this >>>>>> to the jdk8u-hs-dev repository. >>>>>> >>>>>> Thanks, >>>>>> /Staffan >>>>>> >>>>> >>>> >>> >>> >>> -- >>> Dmitry Samersoff >>> Oracle Java development team, Saint Petersburg, Russia >>> * I would love to change the world, but they won't give me the sources. >> > > > -- > Dmitry Samersoff > Oracle Java development team, Saint Petersburg, Russia > * I would love to change the world, but they won't give me the sources.
