You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/08/10 22:49:30 UTC
[GitHub] [lucene-solr] madrob opened a new pull request #1734: LUCENE-9453 Add sync around volatile write
madrob opened a new pull request #1734:
URL: https://github.com/apache/lucene-solr/pull/1734
checkoutAndBlock is not synchronized, but has a non-atomic write to numPending. Meanwhile, all of the other writes to numPending are in sync methods.
In this case it turns out to be ok because all of the code paths calling this method are already sync:
`synchronized doAfterDocument -> checkout -> checkoutAndBlock`
`checkoutLargestNonPendingWriter -> synchronized(this) -> checkout -> checkoutAndBlock`
If we make synchronized checkoutAndBlock that protects us against future changes, shouldn't cause any performance impact since the code paths will already be going through a sync block, and will make an IntelliJ warning go away.
Found via IntelliJ warnings.
https://issues.apache.org/jira/browse/LUCENE-9453
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] s1monw commented on pull request #1734: LUCENE-9453 Add sync around volatile write
Posted by GitBox <gi...@apache.org>.
s1monw commented on pull request #1734:
URL: https://github.com/apache/lucene-solr/pull/1734#issuecomment-671779004
this lock is left out intentionally to reduce the overhead of acquiring a reentrant lock. We only call this method internally from a method that guarantees the lock is held. I don't think we should add this, for correctness it's not necessary at this point. Adding something like `assert Thread.holdsLock(this);` I'd be ok with.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob merged pull request #1734: LUCENE-9453 Add sync around volatile write
Posted by GitBox <gi...@apache.org>.
madrob merged pull request #1734:
URL: https://github.com/apache/lucene-solr/pull/1734
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org