[Bug 62768] New: OPCPackage#close() method is incorrectly synchronized

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62768

            Bug ID: 62768
           Summary: OPCPackage#close() method is incorrectly synchronized
           Product: POI
           Version: 4.0.0-FINAL
          Hardware: PC
            Status: NEW
          Severity: major
          Priority: P2
         Component: OPC
          Assignee: dev@poi.apache.org
          Reporter: bgrh@mail.ru
  Target Milestone: ---

Created attachment 36168
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36168&action=edit
multi-thread runner

There is a code in OPCPackage#close() method, which creates and uses a lock:

ReentrantReadWriteLock l = new ReentrantReadWriteLock();
                try {
                        l.writeLock().lock();
...
} finally {
  l.writeLock().unlock();
}

However, it is completely useless since the *new* lock object is always created
for any method execution, thus voiding this lock.

Suggested change - either move l to a class field and initialize it in
constructor, or simply use synchronized section as read/write lock is not
really needed here.

I attached the sample which runs close() method from multiple thread - as you
can see, they are not synchronized with each other.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

[Bug 62768] OPCPackage#close() method is incorrectly synchronized

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62768

--- Comment #3 from Danila Galimov <bg...@mail.ru> ---
Yes, I agree, maybe the synchronization should be removed at all, but the
classes should be annotated that they are designed to be used in single-thread
environment only.

I just opened the file and found dummy synchronization which won't work anyway,
that's why i reported this.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

[Bug 62768] OPCPackage#close() method is incorrectly synchronized

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62768

--- Comment #2 from Dominik Stadler <do...@gmx.at> ---
I am wondering why we even use synchronization here at all. 

Our thread-safety guarantee does not allow to use one workbook/doc/item in more
than one thread at a time anyway. So this synchronization is not really
necessary as we ask our users to perform synchronization at an upper level
anyway, if they want to access documents across different threads...

Having all of POI fully thread-safe would require many many points of
synchronization and would cause a performance penalty for many current
power-users.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

[Bug 62768] OPCPackage#close() method is incorrectly synchronized

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62768

PJ Fanning <fa...@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #1 from PJ Fanning <fa...@yahoo.com> ---
https://svn.apache.org/repos/asf/poi/trunk@1842142

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org

[Bug 62768] OPCPackage#close() method is incorrectly synchronized

[bu...@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=62768

--- Comment #4 from PJ Fanning <fa...@yahoo.com> ---
I'll remove the synchronized keyword

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org