You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "viliam-durina (via GitHub)" <gi...@apache.org> on 2026/03/16 14:37:48 UTC
[PR] Fix thread notification in `ControlledRealTimeReopenThread` [lucene]
viliam-durina opened a new pull request, #15830:
URL: https://github.com/apache/lucene/pull/15830
`Condition.signal/await` was used to wake up the background thread to reload data. Since the background thread isn't owning the condition's lock at all times, notification can be lost. Specifically, if a new waiter arrives while doing `maybeRefreshBlocking()` and before it obtains the lock again, the thread will go to sleep, instead of refreshing again. Similarly, the thread won't terminate if `close()` is called during this window. The issue is pronounced if minTime is 0 and maxTime is high. Owning the lock all the time is not a solution, because it would unnecessarily block all waiters, even if they are not interested in the generation being loaded.
This PR replaces this notification with a semaphore. The semaphore collects permits, and the background thread tries to acquire a permit with a timeout as a way of sleeping. Releasing a permit serves as the signal. This way no notification is lost.
There's another wait-notify construct in this class, but that one is correct. The waiter owns the lock all the time, and the write to `searchingGen` is done while holding the same monitor.
Other changes included in this PR:
- simplify the background thread loop (the `run()` method), now it's simple steps: loop until finished, in each iteration determine sleep time, if >0, sleep, else refresh.
- move `refreshStartGen` to the listener class, it's used only there
- clarify javadocs: This class is the `Thread` itself, not a utility. The user has to run it. And more...
--
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: issues-unsubscribe@lucene.apache.org
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
Re: [PR] Fix possible lost signal in `ControlledRealTimeReopenThread` [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss merged PR #15830:
URL: https://github.com/apache/lucene/pull/15830
--
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: issues-unsubscribe@lucene.apache.org
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
Re: [PR] Fix possible lost signal in `ControlledRealTimeReopenThread` [lucene]
Posted by "viliam-durina (via GitHub)" <gi...@apache.org>.
viliam-durina commented on PR #15830:
URL: https://github.com/apache/lucene/pull/15830#issuecomment-4212360423
Not synchronizing on the instance itself is a good practice, so it's fine.
Using `Duration` - it's a breaking change, if that doesn't matter, it's fine.
--
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: issues-unsubscribe@lucene.apache.org
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
Re: [PR] Fix possible lost signal in `ControlledRealTimeReopenThread` [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #15830:
URL: https://github.com/apache/lucene/pull/15830#issuecomment-4212392619
This is clearly marked as an experimental class so I don't think this matters. People will have to adjust (and then at least they'll know what the time unit passed to this constructor is - the fraction-of-a-second-double is really strange, at least to me).
--
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: issues-unsubscribe@lucene.apache.org
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
Re: [PR] Fix possible lost signal in `ControlledRealTimeReopenThread` [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15830:
URL: https://github.com/apache/lucene/pull/15830#issuecomment-4159048264
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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: issues-unsubscribe@lucene.apache.org
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
Re: [PR] Fix possible lost signal in `ControlledRealTimeReopenThread` [lucene]
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #15830:
URL: https://github.com/apache/lucene/pull/15830#issuecomment-4205020185
I allowed myself to add an explicit monitor and change the parameters to an explicit Duration. I think it's clearer and the class is experimental so no problem there. Let me know if this is ok.
--
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: issues-unsubscribe@lucene.apache.org
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