Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SessionTracker memory leak on WebSockets that close immediately #6602

Closed
acdcjunior opened this issue Aug 11, 2021 · 1 comment · Fixed by #6604
Closed

SessionTracker memory leak on WebSockets that close immediately #6602

acdcjunior opened this issue Aug 11, 2021 · 1 comment · Fixed by #6604
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@acdcjunior
Copy link

Jetty version(s)

Tested on: 11.0.3 and 11.0.6

Java version/vendor (use: java -version)

Tested on:

  • jre/jdk 11.0.10_9 (openj9-0.24.0)
  • openjdk hotspot 11.0.12+7

OS type/version

Tested on: Windows 10, Linux Alpine

Description

Opening a WebSocket that immediately closes leaves a session instance indefinitely on
org.eclipse.jetty.websocket.common.SessionTracker#sessions.

By "a WebSocket that immediately closes" I mean a WebSocketConnectionListener that closes the WebSocket on the onWebSocketConnect method. Example:

// kotlin
object : WebSocketConnectionListener {
    override fun onWebSocketConnect(session: Session?) {
        session?.close(1008, "I am closing you right away because reason")
    }
} // For the full example in context, see code below.

Don't know if it matters, but I am using this "immediately closed WebSocket" because this was the only way I managed to pass a close reasong (a string, for instance) to the client. Any other ways (e.g. throwing an exception) would terminate the connection with status 1006 and no reason.

Root cause analysis

I have tried to inspect the code a bit to find out what could be causing the leak. From what I could understand, the problem is the SessionTracker not removing the sessions for the "immediately-closed-WebSockets" (ICWS) due to a race condition between its session opened and session closed methods:

Workaround

I have managed to contain the leak by registering an aditional WebSocketSessionListener that, for every opened session, checks if it is closed. If it is, the listener then removes the session from SessionTracker#sessions via reflection.

Workaround code is commented out in the snippet below.

How to reproduce?

// kotlin
import jakarta.servlet.ServletContext
import org.eclipse.jetty.server.Server
import org.eclipse.jetty.server.ServerConnector
import org.eclipse.jetty.servlet.ServletContextHandler
import org.eclipse.jetty.websocket.api.Session
import org.eclipse.jetty.websocket.api.WebSocketConnectionListener
import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer
import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer


fun main() {
    val server = Server()

    val connector = ServerConnector(server)
    connector.port = 8080
    server.addConnector(connector)

    val context = ServletContextHandler(ServletContextHandler.SESSIONS)
    context.contextPath = "/"

    server.handler = context

    JettyWebSocketServletContainerInitializer.configure(context) { _: ServletContext?, wsContainer: JettyWebSocketServerContainer ->
        wsContainer.addMapping("/ws") { _, _ ->
            object : WebSocketConnectionListener {
                override fun onWebSocketConnect(session: Session?) {
                    session?.close(1008, "I am a WS that closes immediately")
                }
            }
        }

        // workaround
//        wsContainer.addSessionListener(object : WebSocketSessionListener {
//            override fun onWebSocketSessionOpened(session: Session?) {
//                if (!session!!.isOpen) {
//                    val sessionTracker = wsContainer.javaClass.getDeclaredField("sessionTracker")
//                    sessionTracker.isAccessible = true
//                    (sessionTracker.get(wsContainer) as SessionTracker).onWebSocketSessionClosed(session)
//                }
//            }
//        })
    }

    server.start()
    server.join()
}

Run main() above and open many connections:

// javascript
new WebSocket("http://localhost:8080/ws")
new WebSocket("http://localhost:8080/ws")
new WebSocket("http://localhost:8080/ws")

By inspecting org.eclipse.jetty.websocket.common.SessionTracker#sessions, you'll see the sessions are held even after whe WebSockets are closed (to inspect it, put a breakpoint on SessionTracker#onWebSocketSessionOpened or SessionTracker#onWebSocketSessionClosed and try to connect a ws).

To use the workaround, uncomment the code and run again. Notice the sessions aren't stacked up anymore.

The code is in Kotlin just because it was the language I was using and it was simpler for me to elaborate the snippet. If you want, let me know and I can convert it to Java.

@acdcjunior acdcjunior added the Bug For general bugs on Jetty side label Aug 11, 2021
@joakime joakime added this to To do in Jetty 10.0.7/11.0.7 FROZEN via automation Aug 11, 2021
@lachlan-roberts
Copy link
Contributor

@acdcjunior thanks for the detailed report.
I think this should be relatively straightforward to fix, I will put up a PR.

lachlan-roberts added a commit that referenced this issue Aug 12, 2021
…n closed in OnOpen

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Jetty 10.0.7/11.0.7 FROZEN automation moved this from To do to Done Aug 16, 2021
lachlan-roberts added a commit that referenced this issue Aug 16, 2021
…rLeak

Issue #6602 - Do not leak WebSocket SessionTracker if closed in onOpen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants