Skip to content

[monitorlib] Close idle connection after a while#1408

Merged
BenjaminPelletier merged 2 commits intointeruss:mainfrom
Orbitalize:fix_1407_v1
Apr 8, 2026
Merged

[monitorlib] Close idle connection after a while#1408
BenjaminPelletier merged 2 commits intointeruss:mainfrom
Orbitalize:fix_1407_v1

Conversation

@the-glu
Copy link
Copy Markdown
Contributor

@the-glu the-glu commented Apr 2, 2026

Proposition 1 to fix connections leaking. See #1409 for alternative version (or we could have both, but that probably not needed).

The test suite is currently creating a set of UTMClientSession during initialization, for each test and endpoint used.

This create issues, as connections use keep-alive mechanism and are never closed. #1407 is an example of that with a lot USS, reaching the limit after a while.

This version of the fix add a thread that close the session after inactivity. This ensure connections are closed and don't share session between tests, but has the disadvantage of creating lot of threads, and therefor background processing outside tests. (NB: We can also use a single thread and a form of register, but that we just remove thread's overhead).

Tested manually by checking the number of open sockets and a very shot timeout that stay low. Connections are still reused.

Copy link
Copy Markdown
Member

@BenjaminPelletier BenjaminPelletier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this approach creates a memory leak. How about something like this approach?

class Foo:
    _timer: threading.Timer | None = None

    def __init__(self):
        Foo._start_closure_timer(weakref.ref(self))

    @staticmethod
    def _start_closure_timer(wref: weakref.ReferenceType[Foo]) -> None:
        def wrapper():
            weak_self_final = wref()
            if weak_self_final is not None:
                try:
                    print("weak_self_final.close()")
                finally:
                    Foo._start_closure_timer(wref)

        weak_self_initial = wref()
        if weak_self_initial:
            weak_self_initial._timer = threading.Timer(SOCKET_KEEP_ALIVE_LIMIT, wrapper)
            weak_self_initial._timer.daemon = True
            weak_self_initial._timer.start()

    def __del__(self):
        if timer := self._timer:
            timer.cancel()

@BenjaminPelletier BenjaminPelletier changed the title [monitorlib] Close idle connexion after a while [monitorlib] Close idle connection after a while Apr 6, 2026
This suggestion implements my comments on
[1408](interuss#1408) plus a few
additional things:
* Avoids memory leak by using weakref in periodic closure check
* Avoids race condition between closure and new request by adding
_closure_lock
* Informs user when automatic closure occurs via log message, including
differentiating UTMClientSession instances with int ID
* Increases keepalive period to just under max keepalive time likely
used by external infrastructure

Tested in current state and verified a large number of sessions (90+)
and closures. Tested cherrypicking
[1411](interuss#1411) and verified a
smaller number of sessions and closures.
@BenjaminPelletier BenjaminPelletier merged commit a8acd6b into interuss:main Apr 8, 2026
22 checks passed
github-actions bot added a commit that referenced this pull request Apr 8, 2026
Co-authored-by: Benjamin Pelletier <BenjaminPelletier@users.noreply.github.com> a8acd6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants