Skip to content

Refactor use or close lock in ConcurrentWebSocketSessionDecorator #36909

Description

@rstoyanchev

The close lock was originally in checkSessionLimits only (ffac748) to prevent concurrent checks from raising an exception and attempting to close the session, which could write a close frame as with SockJS.

The lock was later added to close() (09a40b8), but after a deadlock with a server lock surfaced, the use of the lock in close() was relaxed to tryLock (d151931). However, that means a direct call to close could fail to obtain the lock due to a concurrent limits check from a sending thread.

At present, the close lock no longer needs to protect checkSessionLimits. The resulting SessionLimitExceededException is handled in StompSubProtocolHandler and SubProtocolWebSocketHandler to close the session. The close method itself is protected by the lock and a closeInProgress flag furtherensures a single pass. The close method changes the status to SESSION_NOT_RELIABLE if limits are exceeded, and that means the SockJS layer won't write a close frame.

By no longer using the close lock in checkSessionLimits, the tryLock in close now should work as expected. In addition the sendMessage method will fail consistently when limits are exceeded and the message wasn't sent.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

in: webIssues in web modules (web, webmvc, webflux, websocket)type: bugA general bug

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions