On Mon, 15 Dec 2025 10:52:40 GMT, Daniel Fuchs <[email protected]> wrote:
>> I'd rather not call this from compute; every method called from the compute >> lambda increases the risk of reintroducing the deadlock, so I'd like to keep >> the lambda to a minimum. >> >> Most of the time the connection will be added; the only case where it won't >> is when there are multiple threads attempting to close the connection in >> parallel. The timer task only removes the connection from the endpoint, so, >> worst case, we will remove the connection IDs and the reset tokens twice. >> The second removal will likely be a no-op, unless we somehow manage to >> reassign the IDs or the tokens to a different connection. > > A look at `startTimer` lets me think that it should be safe, but OK. I > suspect the double removal would not be an issue - IIRC we check with == on > the removed connection, but we would be potentially adding an event to the > timer queue which will wake up the timer queue for nothing. We could add a > boolean field to ClosingConnection that we could set to `true` if the > connection is added and check that after compute has been called, and start > the timer then. I'll let you decide if it's worth it. We don't check with ==; technically we could, but with multiple threads accessing the map, I'm not sure if we would guarantee that all connection IDs are unmapped when the connection is removed. Multiple threads racing to update the connection map should be rare. Most of the time the compute calls will replace the connection. so the extra check is probably not worth the effort. I'll move the startTimer call after the connection map updates; I observed occasional failures to update the map, because the timer fired before the map was updated. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28788#discussion_r2624049196
