Skip to content

fix(core): memory leaks and resource management issues #114

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.

Sign up for GitHub

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

Closed
Rushaway wants to merge 1 commit into from
Closed

fix(core): memory leaks and resource management issues #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/torchlight/AudioClip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ def OnStop(self) -> None:
),
)

del self.audio_player

def OnUpdate(self, old_position: int, new_position: int) -> None:
delta = new_position - old_position
self.last_position = new_position
Expand Down
2 changes: 1 addition & 1 deletion src/torchlight/ClientProtocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def data_received(self, data: bytes) -> None:
self.buffer += data

chunks = self.buffer.split(b"\0")
if data[-1] == b"\0":
if data[-1] == 0:
chunks = chunks[:-1]
self.buffer = bytearray()
else:
Expand Down
2 changes: 1 addition & 1 deletion src/torchlight/FFmpegAudioPlayer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def Stop(self, force: bool = True) -> bool:
self.uri = ""

self.Callback("Stop")
del self.callbacks
self.callbacks = []

return True

Expand Down
5 changes: 4 additions & 1 deletion src/torchlight/SourceRCONServer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ def Remove(self, peer: SourceRCONClient) -> None:
self.logger.info(sys._getframe().f_code.co_name + f" Peer {peer.name} disconnected!")
self.peers.remove(peer)

def Close(self) -> None:
self._serv_sock.close()

async def _server(self) -> None:
while True:
Comment on lines +29 to 33

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

Close() is added but not called anywhere, so the listening socket will still remain open unless callers explicitly invoke it. Also, _server() runs an infinite accept loop; closing _serv_sock from another context will typically cause sock_accept to raise and the task to terminate with an exception unless it’s caught/handled. Consider wiring Close() into shutdown logic and updating _server() to exit cleanly on CancelledError/OSError when the socket is closed.

Copilot uses AI. Check for mistakes.
peer_socket: socket.socket
Expand All @@ -39,7 +42,7 @@ async def _server(self) -> None:
self.password,
self.torchlight_handler.command_handler,
)
asyncio.Task(self._peer_handler(peer))
asyncio.ensure_future(self._peer_handler(peer))
self.peers.append(peer)
Comment on lines +45 to 46

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

_peer_handler() is started with asyncio.ensure_future(...) but the returned Task isn’t tracked. If you later implement Close()/shutdown, you won’t be able to cancel or await these per-peer tasks, and unexpected exceptions will be reported as “Task exception was never retrieved”. Consider using asyncio.create_task(...) (since you’re already in the event loop) and storing tasks (e.g., in a set) so they can be cancelled/awaited during shutdown.

Copilot uses AI. Check for mistakes.
self.logger.info(sys._getframe().f_code.co_name + f" Peer {peer.name} connected!")

Expand Down
4 changes: 2 additions & 2 deletions src/torchlight/TorchlightHandler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(self, loop: asyncio.AbstractEventLoop | None, config: Config):

self.Init()

asyncio.ensure_future(self._Connect(), loop=self.loop)
asyncio.ensure_future(self._Connect())

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

TorchlightHandler accepts an explicit event loop (self.loop), but _Connect() is now scheduled via asyncio.ensure_future(self._Connect()), which uses the current event loop rather than self.loop. If a non-default loop is passed (or if self.loop isn’t set as current), this will schedule the task on the wrong loop or fail. Prefer self.loop.create_task(self._Connect()) to ensure the task is bound to the handler’s loop.

Copilot uses AI. Check for mistakes.

async def _Connect(self) -> None:
# Connect to API
Expand Down Expand Up @@ -137,7 +137,7 @@ def OnDisconnect(self, exc: Exception | None) -> None:

self.Init()

asyncio.ensure_future(self._Connect(), loop=self.loop)
asyncio.ensure_future(self._Connect())

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

Same issue as in __init__: asyncio.ensure_future(self._Connect()) ignores self.loop and can schedule onto a different event loop than the one stored on the handler. Use self.loop.create_task(self._Connect()) (or otherwise ensure the intended loop is current) so reconnects are consistently scheduled on the correct loop.

Copilot uses AI. Check for mistakes.

def __del__(self) -> None:
self.logger.debug("~TorchlightHandler()")
2 changes: 1 addition & 1 deletion src/torchlight/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def cli(config_folder: str) -> None:
config["TorchRCON"],
torchlight_handler,
)
asyncio.Task(rcon_server._server())
asyncio.ensure_future(rcon_server._server())

Copilot AI Mar 9, 2026

Copy link

Choose a reason for hiding this comment

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

rcon_server._server() is scheduled with asyncio.ensure_future(...) but the returned Task isn’t stored, and there’s no corresponding shutdown path to cancel the server task and close the listening socket. This makes the new SourceRCONServer.Close() ineffective and can leave the accept-loop running until process exit (or raise noisy exceptions if the socket is closed). Consider storing the created Task and invoking task.cancel() + rcon_server.Close() from graceful_shutdown, or creating the task via event_loop.create_task(...) so it’s clearly bound to event_loop.

Copilot uses AI. Check for mistakes.

# Run!
event_loop.run_forever()