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

[Langer, Christoph]

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