RFR: JDK-8241883: (zipfs) SeekableByteChannel:close followed by SeekableByteChannel:close will throw an NPE

[Alan Bateman]

On 08/04/2020 21:40, Lance Andersen wrote:
>
>
>> On Apr 8, 2020, at 4:26 PM, Alan Bateman <Alan.Bateman at oracle.com 
>> <mailto:Alan.Bateman at oracle.com>> wrote:
>>
>>
>>
>> On 08/04/2020 20:36, Lance Andersen wrote:
>>> :
>>>
>>> ——————————
>>> $ hg diff
>>> *diff -r f4c174bf0276 
>>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java*
>>> *--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java 
>>>     Tue Apr 07 09:03:05 2020 -0400*
>>> *+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java 
>>>     Wed Apr 08 11:07:30 2020 -0400*
>>> @@ -876,6 +876,7 @@
>>>      // channel is closed
>>>      private class EntryOutputChannel extends ByteArrayChannel {
>>>          final Entry e;
>>> +        final ReadWriteLock rwlock = new ReentrantReadWriteLock();
>>>
>>> EntryOutputChannel(Entry e) {
>>>              super(e.size > 0? (int)e.size : 8192, false);
>>> @@ -892,11 +893,24 @@
>>>
>>>          @Override
>>>          public void close() throws IOException {
>>> -            // will update the entry
>>> -            try (OutputStream os = getOutputStream(e)) {
>>> - os.write(toByteArray());
>>> +            beginWrite();
>>> +            try {
>>> + if(!isOpen())
>>> + return;
>>> +                // will update the entry
>>> +                try (OutputStream os = getOutputStream(e)) {
>>> + os.write(toByteArray());
>>> +                }
>>> + super.close();
>>> +            } finally {
>>> + endWrite();
>>>              }
>>> - super.close();
>>> +        }
>>> +        private final void beginWrite() {
>>> + rwlock.writeLock().lock();
>>> +        }
>>> +        private final void endWrite() {
>>> + rwlock.writeLock().unlock();
>>>          }
>>>
>> That is the equivalent of close synchronizing on any object. Are 
>> there any code paths use the readLock?
>
> In the case of the close method above, ByteArrayChannel::toByteArray 
> uses a readLock  ByteArrayChannel::close uses a writeLock
>
The patch adds a new lock and doesn't appear use the readLock. Would it 
be possible to generate a webrev with the full patch as it's not 
possible to see the fully implications of what is proposed here.

-Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200409/8f0919ba/attachment.htm>