RFR: 7181793: Socket getInputStream/getOutputStream create streams that cannot be GC'ed until Socket is closed

[Chris Hegarty]

Thanks Sean. Looks fine to me.

-Chris.

On 09/10/2012 15:47, Seán Coffey wrote:
> Neater approach to testcase alright. Updated :
> http://cr.openjdk.java.net/~coffeys/webrev.7181793.jdk8.02/
>
> I'll leave the push for another few hours just in case others want to
> review. (BTW, I've run the relevant regression tests & JCK tests on this
> change. No issues spotted)
>
> regards,
> Sean.
>
> On 09/10/2012 14:48, Alan Bateman wrote:
>> On 09/10/2012 14:34, Seán Coffey wrote:
>>> Requesting a review for this bug which cropped up whilst cleaning up
>>> the FileDescriptor & associated streams some time back (7105952)
>>>
>>> Turns out that each call to a socket.getOutputStream() creates a new
>>> instance of SocketOutputStream. I'm not seeing any reason to why that
>>> code exists. One instance should be ok here. The getInputStream()
>>> method doesn't have this issue.
>>>
>>> bug report : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7181793
>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.7181793.jdk8/
>>> <http://cr.openjdk.java.net/%7Ecoffeys/webrev.7181793.jdk8/>
>> I think the change is okay. I can't think of why someone would expect
>> a new output stream each time.
>>
>> For the test then I don't think you need a background thread to accept
>> the connection, you could do this:
>>
>> try (ServerSocket ss = new ServerSocket(0)) {
>> try (Socket s = new Socket(...); Socket peer = ss.accept()) {
>> for (int i=0; i<100000; i++) { ...}
>> }
>> }
>>
>> That will also ensure that all sockets are closed (although it
>> probably doesn't matter here because the tests runs in its own VM).
>>
>> -Alan.
>>
>>
>>
>>
>