Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Possible Thread Leak in ... #243

Closed
thomaschristopherking opened this issue Dec 16, 2021 · 1 comment
Closed

Possible Thread Leak in ... #243

thomaschristopherking opened this issue Dec 16, 2021 · 1 comment

Comments

@thomaschristopherking
Copy link

thomaschristopherking commented Dec 16, 2021

Description

The number of threads increases with each transaction in some cases (hard to replicate exactly). This is an example thread's stack trace, all of the unexpected and additional threads have the same trace:

["\n# ThreadID: 139762757248768","File: \"/usr/lib/python3.8/threading.py\", line 890, in _bootstrap"," self._bootstrap_inner()","File: \"/usr/lib/python3.8/threading.py\", line 932, in _bootstrap_inner"," self.run()","File: \"/usr/lib/python3.8/threading.py\", line 870, in run"," self._target(*self._args, **self._kwargs)","File: \"/home/ubuntu/adam-action-graph-service/action_selection/venv/lib/python3.8/site-packages/typedb/stream/request_transmitter.py\", line 108, in run"," self._permission_to_run.acquire()","File: \"/usr/lib/python3.8/threading.py\", line 433, in acquire"," self._cond.wait(timeout)","File: \"/usr/lib/python3.8/threading.py\", line 302, in wait"," waiter.acquire()"]

Environment

  1. Ubuntu
  2. TypeDB 2.5.0
  3. TypeDB client-python 2.5.0
  4. Python version 3.8

Reproducible Steps

Unclear at the moment.

Expected Output

"Request transmitter" threads to be closed at the end of a transaction.

Actual Output

The number of threads grows with each transaction, even once closed.

alexjpwalker added a commit that referenced this issue Jan 25, 2022
## What is the goal of this PR?

Instead of launching a background thread in each session to send pulses, we now use a single thread in the Client to send pulses for all of its sessions. This allows the client to handle a much larger number of sessions than it previously could.

## What are the changes implemented in this PR?

**Changeset**
- Move session pulse transmitter to the Client - fixes an issue where spawning a session always spawned a new thread, and it would stick around for at least 5 seconds
- Fixed a bug where closing many sessions concurrently could throw a concurrent modification error

**Fixes**
- fixes #222
- may reduce the chance of seeing #243
@thomaschristopherking
Copy link
Author

thomaschristopherking commented Feb 17, 2022

It looks like this issue is now resolved, thank you (assuming you want this closed).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants