[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8057558 VirtualMachineImpl.execute on windows should close PipedInputStream before throwing
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2014-09-09 6:51:27
Message-ID: 540EA36F.70807 () oracle ! com
[Download RAW message or body]
On 9/8/14 10:39 PM, Staffan Larsen wrote:
>
> On 8 sep 2014, at 22:03, serguei.spitsyn@oracle.com
> <mailto:serguei.spitsyn@oracle.com> wrote:
>
>> On 9/8/14 12:59 PM, Staffan Larsen wrote:
>>>
>>> On 8 sep 2014, at 21:26, serguei.spitsyn@oracle.com
>>> <mailto:serguei.spitsyn@oracle.com> wrote:
>>>
>>>> This looks good to me.
>>>
>>> Thanks.
>>>
>>>>
>>>> A minor question:
>>>>
>>>> src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java
>>>> 110 is.close();
>>>>
>>>> An IOException can be thrown in the readErrorMessage().
>>>> Would it make sense to use a finally statementto close the stream?
>>> In most cases the stream is returned from the method so we can’t
>>> always close it.
>>
>> In this particular case the stream is not returned from the method call:
>> 103 PipedInputStream is = new PipedInputStream(hPipe);
>
> 123 // return the input stream
> 124 return is;
Ah... I see what you mean.
Sorry for the noise.
Thanks,
Serguei
>
> /Staffan
>
>
>>
>> Thanks,
>> Serguei
>>
>>>
>>> /Staffan
>>>
>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>> On 9/8/14 5:25 AM, Staffan Larsen wrote:
>>>>> Two fixes:
>>>>> - The PipedInputStream used by the attach code on windows is not closed in case of errors
>>>>> - The InputStreams returned by VirtualMachine.execute are not closed by all callers
>>>>>
>>>>> webrev:http://cr.openjdk.java.net/~sla/8057558/webrev.00/
>>>>> bug:https://bugs.openjdk.java.net/browse/JDK-8057558
>>>>>
>>>>> Thanks,
>>>>> /Staffan
>>>>
>>>
>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 9/8/14 10:39 PM, Staffan Larsen
wrote:<br>
</div>
<blockquote
cite="mid:340850CA-DEEA-48DA-A372-1EDBCB2E8842@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<br>
<div>
<div>On 8 sep 2014, at 22:03, <a moz-do-not-send="true"
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 9/8/14 12:59 PM, Staffan
Larsen wrote:<br>
</div>
<blockquote
cite="mid:BF54441F-3A4C-441A-95A0-FB7342CD8221@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<br>
<div>
<div>On 8 sep 2014, at 21:26, <a moz-do-not-send="true"
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">This looks good to me.<br>
</div>
</div>
</blockquote>
<div><br>
</div>
Thanks.</div>
<div><br>
<blockquote type="cite">
<div text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix"> <br>
A minor question:<br>
<br>
src/jdk.attach/windows/classes/sun/tools/attach/VirtualMachineImpl.java<br>
<meta http-equiv="content-type"
content="text/html; charset=windows-1252">
<pre><span class="new"> 110 is.close();
An IOException can be thrown in the readErrorMessage().
Would it make sense to use a finally statement </span><span class="new"><span \
class="new">to close the stream</span>? </span></pre>
</div>
</div>
</blockquote>
<div>In most cases the stream is returned from the
method so we can’t always close it.</div>
</div>
</blockquote>
<br>
In this particular case the stream is not returned from the
method call:<br>
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<pre> 103 PipedInputStream is = new \
PipedInputStream(hPipe);</pre> </div>
</blockquote>
<div><br>
</div>
<pre style="background-color: rgb(238, 238, 238); position: static; z-index: \
auto;">123 // return the input stream 124 return is;</pre>
</div>
</blockquote>
<br>
Ah... I see what you mean.<br>
Sorry for the noise.<br>
<br>
Thanks,<br>
Serguei<br>
<blockquote
cite="mid:340850CA-DEEA-48DA-A372-1EDBCB2E8842@oracle.com"
type="cite">
<div>
<div><br>
</div>
<div>/Staffan</div>
<div><br>
</div>
<div><br>
</div>
<blockquote type="cite">
<div text="#000000" bgcolor="#FFFFFF"> <br>
Thanks,<br>
Serguei<br>
<br>
<blockquote
cite="mid:BF54441F-3A4C-441A-95A0-FB7342CD8221@oracle.com"
type="cite">
<div>
<div><br>
</div>
<div>/Staffan</div>
<div><br>
</div>
<br>
<blockquote type="cite">
<div text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">
<pre><span class="new">
Thanks,
Serguei
</span></pre>
<br>
On 9/8/14 5:25 AM, Staffan Larsen wrote:<br>
</div>
<blockquote
cite="mid:AC533746-8B62-40FC-BE59-3C568A32CB4B@oracle.com"
type="cite">
<pre wrap="">Two fixes:
- The PipedInputStream used by the attach code on windows is not closed in case of \
errors
- The InputStreams returned by VirtualMachine.execute are not closed by all callers
webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Esla/8057558/webrev.00/">http://cr.openjdk.java.net/~sla/8057558/webrev.00/</a>
bug: <a moz-do-not-send="true" class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8057558">https://bugs.openjdk.java.net/browse/JDK-8057558</a>
Thanks,
/Staffan</pre>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic