[10.4-stable] backport: httputil: fix potential memleak #3713
Conversation
IMHO similar to 9c194dd the memleak also has to be fixed here Signed-off-by: Christoph Ostarek <christoph@zededa.com>
|
We'll try to reproduce/verify with a huge application used to trigger a mem leak before. |
|
@rene, @christoph-zededa, @milan-zededa, so, I guess, we should switch it to the "ready to review" state? ) |
copied from lfedge/eve-libs:307499b404872ab07d0ec20c59cdf7a8c7cd6936 Signed-off-by: Christoph Ostarek <christoph@zededa.com>
|
I added another commit, porting lf-edge/eve-libs@307499b to here |
|
The second leak! =) |
There was a problem hiding this comment.
This indeed fixes the described leaks, but in order to see inactivityTimer leak in action you need to request download of thousands of images in 5 minute, which seems is not the case. @christoph-zededa please backport the inactivityTimer fix to all the branches to keep the code same.
The socket leak is nasty, but I doubt you hit the memory pressure before you hit the limit of number of opened file descriptors (which I assume not a huge number, at least I can't find that we call the setrlimit(RLIMIT_NOFILE) in the code)
Anyway, good catch
christoph-zededa
changed the title
|
Converting to draft, first have to find out how to do a proper |
Did: cp libs/zedUpload/httputil/http.go pkg/pillar/vendor/github.com/lf-edge/eve/libs/zedUpload/httputil/http.go as make proto && make proto-vendor does not have an impact Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Please see 59677cd |
Backport for 9.4-stable is here: #3722 11.0-stable go.mod is here: https://github.com/lf-edge/eve/blob/11.0-stable/pkg/pillar/go.mod#L28 |
There was a problem hiding this comment.
But shouldn't we fix the version in go.mod as well? Not it points to github.com/lf-edge/eve/libs v0.0.0-20230717153241-cc5629af4657 and if I run go mod vendor it rolls the lib file in the vendor directory back to the older version...
Sorry if I'm asking a stupid question, I'm still confused with the go modules stuff.
|
Argh... Discussed it with @christoph-zededa offline. As I understand it now, the algorithm is the following:
I remember we discussed it before with @milan-zededa and @deitch, but I'm always forgetting that... |
IMHO similar to 9c194dd the memleak also has to be fixed here
The fix for a newer version has also been introduced here: lf-edge/eve-libs@127266b#diff-4a3ca89f1c1037aa18b0258facf633f93de2016bfb09906794e7cc7c88f4fd62R264 (which has then been replaced by lf-edge/eve-libs@08a38c8 )