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

Fix Gun Message Leaks #659

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

humaite
Copy link
Collaborator

@humaite humaite commented Dec 2, 2024

ar_http client was using internal functions
from inet (inet:start_timer/1 and inet:stop_timer/1). Those functions are wrapper around timer module and send a message to the current active process doing the http request. Unfortunately, this is not compatible with `gun:await/3è function and breaks its loop. Gun messages are after send to the active process.

see: https://github.com/ArweaveTeam/arweave-dev/issues/717

@humaite humaite self-assigned this Dec 2, 2024
@humaite humaite force-pushed the humaite/717-unhandled-message-in-blacklist branch 2 times, most recently from efea96e to e7042b4 Compare December 3, 2024 07:56
ar_http client was using internal functions
from inet (`inet:start_timer/1` and `inet:stop_timer/1`).
Those functions are wrapper around `timer` module and
send a message to the current active process doing the
http request. Unfortunately, this is not compatible with
gun:await/3 function and breaks its loop. Gun messages
are after send to the active process.

This commit also enforce request cancellation when
something goes wrong (error/timeout) to avoid leaking
messages coming in the current process.
@humaite humaite force-pushed the humaite/717-unhandled-message-in-blacklist branch from e7042b4 to 33b2be2 Compare December 3, 2024 07:58
@humaite
Copy link
Collaborator Author

humaite commented Dec 3, 2024

After more than 10 hours running on a test server, this patch looks okay. No leaks reported by logs.

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

Successfully merging this pull request may close these issues.

1 participant