Request for review : 7181793 FileDescriptor keeps its own hard reference to Closeables causing a memory leak

[Seán Coffey]

>
> As regards 7u6 then changing java.net.Socket is too risky to do this 
> late in the release so I agree with your proposal to back-out  
> 7105952. It's probably best to create a separate bug for this so that 
> the history is easy to understand. We can then use 7181793 to fix 
> java.net.Socket in jdk8 (and maybe a future 7u). To keep things simple 
> then I think the change to anti-delta the fix should be just that and 
> shouldn't be complicated at adding in a new test. The fix for 7105952 
> can introduce additional tests for this area. 
I just logged a new CR to track the backout change Alan.
CR 7183209 : Backout 7105952 changes for jdk7u

Regards,
Sean.

On 11/07/12 11:43, Alan Bateman wrote:
> On 10/07/2012 17:37, Seán Coffey wrote:
>>
>> 7105952 fix introduced some improvements for finalization strategy 
>> around FileInputStream/FileOutputStream/RandomAccessFile.
>>
>> However a recently reported issue has highlighted an issue where 
>> memory heap can be consumed with SocketOutputStream objects as a 
>> result of clients repeatedly calling socket.getOutputStream() on a 
>> common socket (shared fd).
>>
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7181793
>>
>> Use of weakreferences was considered at 7105952 fix time but hard 
>> references was considered a cleaner approach. Altering that design at 
>> this late stage in the 7u6 plan could be risky and I'm suggesting 
>> that we back out the 7105952 fix and concentrate on this area again 
>> in 7u8.
>>
>> The changeset is basically a backout of the original changeset plus 
>> the addition of a new testcase (FDStrongReference.java)
>>
>> http://cr.openjdk.java.net/~coffeys/webrev.7181793.jdk7u6/ 
>> <http://cr.openjdk.java.net/%7Ecoffeys/webrev.7181793.jdk7u6/>
>>
>> Once reviewed, I'll ping 7u openjdk alias to start the critical fix 
>> approval process.
>> regards,
>> Sean.
> I think the changes for 7105952 are okay, it's just that they have 
> exposed an issue with java.net.Socket's getInputStream and 
> getOutputStream methods. These methods should really be changed to 
> that they only create one InputStream or OutputStream per Socket, not 
> a stream connected to the FileDescriptor for each call to 
> getInputStream or getOutputStream. Clearly changing this would involve 
> a subtle behavior change so it may be better off to keep that for a 
> major release (meaning 8).
>
> On weak references then this isn't going to work for FileInputStream 
> or FileOutputStream because their finalize methods are specified to 
> close the stream. This topic has a long history with several attempts 
> to fix all the issues going back over several releases.
>
> As regards 7u6 then changing java.net.Socket is too risky to do this 
> late in the release so I agree with your proposal to back-out  
> 7105952. It's probably best to create a separate bug for this so that 
> the history is easy to understand. We can then use 7181793 to fix 
> java.net.Socket in jdk8 (and maybe a future 7u). To keep things simple 
> then I think the change to anti-delta the fix should be just that and 
> shouldn't be complicated at adding in a new test. The fix for 7105952 
> can introduce additional tests for this area.
>
> -Alan.