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

Avoid thread leak and data race in cleanUpAsync #432

Merged
merged 1 commit into from
Apr 19, 2023
Merged

Conversation

vickenty
Copy link
Contributor

A thread started in cleanUpAsync may overlap with Instance#init called from fixBrokenInstances, closing a new freshly created connection object instead of the old one. This is fixed by capturing specific connection object in the thread, not the whole instance.

If a connection issue persists over multiple collection intervals, there will be multiple calls to cleanUpAsync while an Instance references the same connection object. closeConnector may block to wait for a network protocol timeout; and, because the implementation is synchronized, multiple concurrent calls will wait for it sequentially, causing build-up of threads all waiting to close a connection. Reset the Instance connection to null, to make sure only one thread is trying to close a connection.

A thread started in cleanUpAsync may overlap with Instance#init called
from fixBrokenInstances, closing a new freshly created connection
object instead of the old one. This is fixed by capturing specific
connection object in the thread, not the whole instance.

If a connection issue persists over multiple collection intervals,
there will be multiple calls to cleanUpAsync while an Instance
references the same connection object. closeConnector may block to
wait for a network protocol timeout; and, because the implementation
is synchronized, multiple concurrent calls will wait for it
sequentially, causing build-up of threads all waiting to close a
connection. Reset the Instance connection to null, to make sure only
one thread is trying to close a connection.
@jk2l
Copy link

jk2l commented Apr 17, 2024

This changes may have caused #519

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.

3 participants