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

[Lance Andersen]

> On Apr 8, 2020, at 6:12 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 07/04/2020 21:11, Lance Andersen wrote:
>> :
>> 
>> I can look to add some tests going forward as I am not sure we have any existing tests
>> 
> The close method should be idempotent, also should be thread safe in this zipfs implementation to avoid random exceptions. If I read the proposed fix correctly then it still have timing issues when there are several thread accessing the zip file system, is that correct?


Ok, I believe the revised  patch below is in line with what you are suggesting:

——————————
$ 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();
         }
     }
———————————————

Best
Lance
> 
> -Alan

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200408/6d714793/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200408/6d714793/oracle_sig_logo-0001.gif>