On 05/11/2025 00:21, Phil Steitz wrote:
The nice test case provided with pool-424 shows another liveness corner
case. The analysis in the report is correct. Basically, ensureIdle is too
weak for this scenario. The simple fix for the bug is to replace that with
addObject, which resolves the issue and has the right semantics, IMO ("an
eye for an eye"). That is a behavior change though and it breaks a bunch
of other tests. It really is great that we have such complete tests in
[pool]. Other tests expect that invalidateObject does not add new objects
unless there are take waiters. This is not really documented anywhere in
the public API and I personally think that "eye for an eye" semantics on
invalidation is a good thing as it helps with liveness issues such as the
one in this report. A good concrete example is abandoned object removal.
Current behavior is to invalidate *but not replace* abandoned instances.
My proposed change would try to replace them.
The only drawback I can see in this is that while addObject can be counted
on to maintain the maxTotal invariant, it ignores maxIdle, so in some cases
this could result in maxIdle exceeded. I see this as a bug in addObject,
which I would like to add as a separate issue to address before this one.
To be clear, with current code addObject can create idle capacity beyond
maxIdle. I propose to make it a no-op in this case (as it is vis a vis
maxTotal).
Before I push the fix for this issue and adjust the failing tests, I
thought it best to ask if anyone objects to this change.
No objections here.
I took a quick look at alternative approaches and couldn't see a better
one. I agree with your view that the "eye for and eye" semantics are
correct. It would be good to add a comment to that effect in the code.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
