lhotari opened a new issue, #21304: URL: https://github.com/apache/pulsar/issues/21304
PendingAckHandleImpl seems to contain some thread safety issues: individualAckOfTransaction uses org.apache.commons.collections4.map.LinkedMap which is not thread safe. Another thread safety issue is the mutation of MutablePair instance. MutablePair is not thread safe. HashMaps are also used as mutable state. This state is mutated without a guarantee that the same thread is reading and writing the state or there's some other way to ensure consistency according to the Java Memory Model. It seems that the intention would be use a single thread execution model to address this. There's a internalPinnedExecutor in [PendingAckHandleImpl](https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/PendingAckHandleImpl.java) which is used to run some operations on the single executor thread. However, the thread could possibly get switched multiple times in the call chain because of asynchronous method calls. There's also some code where synchronized blocks are used as a way to ensure consistency such as `synchronized (org.apache.pulsar.broker.transaction.pendingack.impl.PendingAckHandleImpl.this) {` and `synchronized (PendingAckHandleImpl.this) {`. This is bad since the thread that gets synchronized is whatever thread is completing the asynchronous call. This might for example block threads from the bookkeeper client. -- 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]
