lhotari commented on issue #14436:
URL: https://github.com/apache/pulsar/issues/14436#issuecomment-1066087068


   There seem to be cases where causality and "happens-before" isn't sufficient 
and classes must be thread safe. 
   
   For example BookKeeper client's LedgerEntriesImpl contains this type of code:
   
https://github.com/apache/bookkeeper/blob/7087fda4e91e9fe8974f9d36e01e275a08bf38f1/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/impl/LedgerEntriesImpl.java#L57-L63
   
   ```
       private void releaseByteBuf() {
           if (entries != null) {
               entries.forEach(LedgerEntry::close);
               entries.clear();
               entries = null;
           }
       }
   ```
   
   If there are multiple threads that call this method, the change to the 
entries field wouldn't be visible to the other thread and this could cause to 
the "double release" problem that might be causing the problem in this issue 
where the logging added by @congbobo184 proved that a single instance was used 
in 2 locations simultaneously.
   
   @congbobo184 would you be able to make an experiment where you make all 
methods synchronized in LedgerEntriesImpl and add a synchronized setter to 
modify the "entries" field? This doesn't mean that it should be the way to 
resolve the issue. 
   By making LedgerEntriesImpl competely thread safe, that would ensure that 
lack of thread safety in LedgerEntriesImpl is a source of problems.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to