Alan, Claes,

If my memory was correct, ExChannelClose was not being used even in previous implementation.

Actually it should never have been used in any of the released JDK. I might have been missing some discussions ... so I might be wrong here. But the original reason of having a exchannelcloser was

that I was implementing the sync() the way that it would be invoked during its lifetime. Means you

can actually fresh the in-memory entries back to underlying zipfile to reduce memory pressure ...

because the fs is not really being closed in this situation, we can't just close those streams. Have to

keep them alive till they get closed by the caller. But then the plan was changed and the sync() is

being invoked only in its close(). In this situation, we should simply close those pending streams.

The filesystem is getting closed, any inputstream open on its entry should be closed as well. No

need for this closer any more in current sync() use scenario.

-Sherman



ExistingChannelCloser

On 4/23/19 4:42 AM, Claes Redestad wrote:
Hi Alan, Christoph,

thanks for reviewing this!

On 2019-04-23 12:42, Alan Bateman wrote:
On 18/04/2019 14:10, Claes Redestad wrote:
Webrev: http://cr.openjdk.java.net/~redestad/8222532/open.00/
I think the approach looks good.

One thing that I think could be improved is ExChannelCloser. I think it needs a better name. Also I think it should encapsulate its fields a lot better so that sync doesn't need to access its interanls -- e.g. maybe it can define a closeAndDelete method that closes the channel and deletes the path.


Based on what it does, ExChannelCloser (which I restored from its pre-
existing state) should probably be named ExistingChannelCloser.

I've collected the feedback from you, Christoph and Lance into this
new webrev: http://cr.openjdk.java.net/~redestad/8222532/open.01/

Incremental: http://cr.openjdk.java.net/~redestad/8222532/open.00_01/

Thanks!

/Claes

Reply via email to