RFR 7143743: (zipfs) Potential memory leak with zip provider

Jaikiran Pai jai.forums2013 at gmail.com
Mon Jan 13 10:31:38 UTC 2020

Hello Christoph,

Setting inodes to null sounds fine to me and as you say since its usage
is already guarded by ensureOpen, IMO, it should be fine. I've now
updated the patch to set inodes to null in close() and the new updated
webrev is at https://cr.openjdk.java.net/~jpai/webrev/7143743/3/webrev/


On 13/01/20 12:26 pm, Langer, Christoph wrote:
> Hi,
> I'm wondering whether we shouldn't just do "inodes = null;"? I guess this is cheaper and accesses to the inodes map on a closed filesystem object should not happen anyway (e.g. all is guarded by ensureOpen()). Other than that, the change looks fine.
> Cheers
> Christoph
>> -----Original Message-----
>> From: nio-dev <nio-dev-bounces at openjdk.java.net> On Behalf Of Jaikiran
>> Pai
>> Sent: Samstag, 11. Januar 2020 11:24
>> To: Alan Bateman <Alan.Bateman at oracle.com>; nio-dev at openjdk.java.net
>> Cc: core-libs-dev at openjdk.java.net
>> Subject: Re: RFR 7143743: (zipfs) Potential memory leak with zip provider
>> Hello Alan,
>> On 11/01/20 3:37 pm, Alan Bateman wrote:
>>> On 11/01/2020 09:51, Jaikiran Pai wrote:
>>>> :
>>>> The commit here fixes that issue by simply clearing the "inodes" map in
>>>> the jdk.nio.zipfs.ZipFileSystem.close() method. I have checked the usage
>>>> of the "inodes" map and from what I see, it's usage in various places is
>>>> guarded by "ensureOpen" checks, which means that once the
>> ZipFileSystem
>>>> instance is closed, the contents of these "inodes" map is no longer
>>>> relevant and hence clearing it shouldn't cause any issues.
>>> Clearing the inodes map should be okay for cases where something is
>>> holding a reference to a closed zip file system. However, you should
>>> look at beginWrite/endWrite so that all access to the map is
>>> consistently synchronized.
>> Thank you very much for that input - I hadn't considered the concurrency
>> aspect of it. Based on your input and after looking at the usage of the
>> "inodes", I have now updated the patch to use proper locks during the
>> clearing of the inodes. The updated webrev is available at
>> https://cr.openjdk.java.net/~jpai/webrev/7143743/2/webrev/
>> -Jaikiran

More information about the nio-dev mailing list