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

[Alan Bateman]

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.