chtompki commented on PR #1475: URL: https://github.com/apache/commons-lang/pull/1475#issuecomment-3451614951
I love you guys!On Oct 27, 2025, at 8:48 AM, Alex Herbert ***@***.***> wrote:aherbert left a comment (apache/commons-lang#1475) I think this is an improvement. However it is difficult to write concurrent code and there may be issues I did not notice. The class seems to have issues around the computation of permits, acquires and limits as it makes some assumptions on the limit based on whether it is zero or negative but then does not guard the limit to the range [0, MAX_VALUE]. Thus the limit can be set to a large negative value and break existing functionality. I noted that the fair flag is not enforced when using the semaphore as it uses tryAcquire() which does not respect fairness. So the current unit test is not robust enough to detect this. Also not changed in the PR, but it broken in the current code is the computation in the method getAvailablePermits(). This method can return negative values which do not make sense. It should return a value in the positive range if there is a configured limit. If the limit is disabled then it should return either -1 as a documented flag or Integer.MAX_VALUE to state that you can have as many permits as you desire. I prefer returning MAX_VALUE so that clients can compare this value to any positive number of permits that they desire. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***> -- 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]
