Skip to content

Conversation

@aryan-212
Copy link
Contributor

@aryan-212 aryan-212 commented Aug 13, 2025

This PR fixes #2
@Matrx123

@Matrx123
Copy link
Owner

Hello Aryan, that was quick!!
one thing though, there is a problem i see with the thread implementation:
you are spawning a new thread each time on send alert and not clearing or dropping the channel which will results in memory leak at some point.
maybe you can create the drop trait to properly close the channel and stop the thread, you can also create a thread at once, and in network monitor (tokio implementation) we used unbounded channel, so here on thread you can use try_send() to send to avoid blocking.

i like your idea to keep the buffer size and write in batches, but we can also clear flush it after it fills , its a small number so you can use usize in place of i32.

i am just thinking out loud here, open for discussions!!
cheers 🥳

@aryan-212
Copy link
Contributor Author

Hello Aryan, that was quick!! one thing though, there is a problem i see with the thread implementation: you are spawning a new thread each time on send alert and not clearing or dropping the channel which will results in memory leak at some point. maybe you can create the drop trait to properly close the channel and stop the thread, you can also create a thread at once, and in network monitor (tokio implementation) we used unbounded channel, so here on thread you can use try_send() to send to avoid blocking.

i like your idea to keep the buffer size and write in batches, but we can also clear flush it after it fills , its a small number so you can use usize in place of i32.

i am just thinking out loud here, open for discussions!! cheers 🥳

Hi, thanks for figuring that out, totally forgot the joining of threads 😅

@Matrx123
Copy link
Owner

Hello aryan,
please share the sshot of logs as well from your local environment in the discussions tab for future refrences.
Thanks for the PR though, i will review and merge.

Cheers!! 🥳

@aryan-212
Copy link
Contributor Author

image Here ! @Matrx123

@Matrx123 Matrx123 merged commit 9fcb4d6 into Matrx123:main Aug 14, 2025
@aryan-212 aryan-212 deleted the fix/save-log branch August 14, 2025 20:48
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.

Feature to save logs to a file

2 participants