[GitHub] [pulsar] casuallc commented on pull request #11813: [ISSUE 11796] throw NPE when readEntry

[GitBox <gi...@apache.org>]

casuallc commented on pull request #11813:
URL: https://github.com/apache/pulsar/pull/11813#issuecomment-916616622


   > > good work @casuallc . The changes in this PR seem to be necessary for handling the failure case that is highlighted.
   > > btw. There seems to be a similar change #11292 in progress, but that is taking a different approach for solving a similar issue. I'd prefer the approach used in this PR over the changes in 11292 since the solution in this PR is very simple.
   > 
   > @lhotari
   > IMHO, there is race condition in this solution.
   > If `op.readPosition = cursor.ledger.startReadOperationOnLedger(readPositionRef, op);` is failed in `OpReadEntry.create`, this OpReadEntry is recycled in `readEntriesFailed`.
   > So if we check `op.readPosition` after `OpReadEntry.create`, this `OpReadEntry` object could be reused in other threads, and maybe the `readPosition` is properly initialized.
   > 
   > That's why I try to init `readPosition` after each `OpReadEntry.create` method in #11292 , causing the complexity.
   
   @Jason918 Yes, you are right. I should check op but not op.readPosition.
   I changed the code, PTAL.


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org