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 Timer Handling and Prevent Memory Leaks #297

Merged
merged 7 commits into from
Feb 7, 2025
Merged

Conversation

iw4p
Copy link
Contributor

@iw4p iw4p commented Feb 6, 2025

Hi! I realized some allocs and goroutine leaks in this library as I am working with it on the production. Using pprof and graphviz I realized the problem is time.After() because it shows time NewTimer
examples: 1 - 2

Screenshot 2025-02-07 at 12 23 40 AM Screenshot 2025-02-07 at 12 24 32 AM

@m0leynik
Copy link
Contributor

m0leynik commented Feb 7, 2025

Seems like the issue could be solved in Go 1.23
https://go.dev/doc/go1.23#timer-changes

@iw4p
Copy link
Contributor Author

iw4p commented Feb 7, 2025

Interesting, Thanks for sharing. Right now this library uses 1.18, I am not sure upgrading the golang version is a better idea or changing time.After() to time.NewTicker()

@xssnick
Copy link
Owner

xssnick commented Feb 7, 2025

Hi, thank you for pr, but in dev v.1.11 i rewrote much logic related to rldp and adnl, so it is a bit irrelevant now :( some parts was removed and some conflicts. Could you please merge it with dev-v1-11?

By the way, if you build using go 1.23 you are getting features from compiler version, go.mod just indicates minimal supported version, so im holding it low

@xssnick xssnick changed the base branch from master to dev-v1-11 February 7, 2025 11:46
@iw4p
Copy link
Contributor Author

iw4p commented Feb 7, 2025

Hi! sure. as you changed the base branch from master to dev-v1-11, I have two options, rebase the dev-v1-11 to my master and add push, or you have to change my base branch from master to dev-v1-11, so you merge my dev-v1-11 with your dev-v1-11.
I'm fine with first one if you are done with dev-v1-11, then I can do rebase and make master update with your changes + my changes.

@iw4p
Copy link
Contributor Author

iw4p commented Feb 7, 2025

Rebased. what's the next step?

@xssnick xssnick merged commit 5925a69 into xssnick:dev-v1-11 Feb 7, 2025
@xssnick
Copy link
Owner

xssnick commented Feb 7, 2025

Thank you, merged

@iw4p
Copy link
Contributor Author

iw4p commented Feb 7, 2025

Your welcome! Is there any estimated time for releasing this new version?

@xssnick
Copy link
Owner

xssnick commented Feb 7, 2025

Will try today

@iw4p
Copy link
Contributor Author

iw4p commented Feb 7, 2025

Hi, thank you for pr, but in dev v.1.11 i rewrote much logic related to rldp and adnl, so it is a bit irrelevant now :( some parts was removed and some conflicts. Could you please merge it with dev-v1-11?

By the way, if you build using go 1.23 you are getting features from compiler version, go.mod just indicates minimal supported version, so im holding it low

@m0leynik The strange part is I am using golang:1.23 but still I have the time.After() problem.

@m0leynik
Copy link
Contributor

m0leynik commented Feb 8, 2025

@iw4p
Not sure I've understood correctly, did you change the compiler version in go.mod for tonutils or just run compilation using Go 1.23 compiler?
If the version in go.mod remained unchanged (1.18), the behavior appears to be expected since:

For packages within the module, the compiler rejects use of language features introduced after the version specified by the go directive

https://go.dev/ref/mod#go-mod-file-go

@xssnick
I noticed this and thought it might be helpful for go.mod compiler version management

@iw4p
Copy link
Contributor Author

iw4p commented Feb 8, 2025

No, I mean the docker image of golang. I didn't change the go.mod version. @m0leynik

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