Sherman,
I actually left the ExChannelCloser out as I was stepping back the
changes one by one, thinking it had nothing to do with the behavior
we're trying to restore here, but then I ran into some obscure test
failures (IIRC some zip files not being generated as expected) - when I
restored the ExChannelCloser code those tests passed.
Thus I'd like to leave it as-is for this patch to keep tests happy, but
I admit I don't fully understand what was going on in those tests. OK
to leave it for an RFE to re-examine and remove the
Ex(isting)ChannelCloser?
/Claes
On 2019-04-24 09:28, Xueming Shen wrote:
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
