turnage / graw

Golang Reddit API Wrapper
MIT License
286 stars 50 forks source link

More memory leak issue? #29

Closed guliyevemil1 closed 7 years ago

guliyevemil1 commented 7 years ago

Hey so, I think there is another problem because my bot is still continually crashing (every 16 hours instead of every 3 hours now). I have a feeling there is a similar kind of leak, but I am unable to find it for some reason and might have to debug my code more systematically than I am. I wanted to create the issue just in case you were aware of something that might be causing this that I'm not seeing.

turnage commented 7 years ago

Can you provide details about your system and a crash report? I can try to reproduce the issue.

guliyevemil1 commented 7 years ago

Well, I'm basically calling Listing over and over with different subreddits each time. I also am calling Run on r/all. The thing I don't understand is that the memory in heap is actually pretty low compared to the total memory usage which seems to continually increase over time. I was thinking that not closing the response body caused the response bytes to stay in memory (which didn't count toward heap usage?) which is why I was seeing this issue and thought that my fix would be the end of it, but it only seems to have been a part of the problem.

turnage commented 7 years ago

Why do you suspect the memory leak is causing your crash?

Some operating systems have a low limit on file descriptors which may explain why leaving response bodies open crashed you. The memory usage should grow; the gc doesn't give memory back to the os when programmers free it.

Can you post a output?

guliyevemil1 commented 7 years ago

Oops, I accidentally closed the issue. It would be quite a coincidence then that my app crashes every time the memory usage gets to the 4GB point which is what I have it limited it to. I realize that the gc doesn't necessarily give the memory back to the os, but the heap usage never actually goes above 1GB, so the memory leak feels like a culprit to me.

turnage commented 7 years ago

It's hard to say without the output of your program. Do you have the stdout and stderr of the crash?

guliyevemil1 commented 7 years ago

It doesn't look like any errors were outputted when the crash happened. I think the process just OOM'd.

guliyevemil1 commented 7 years ago

Ah, I went back to see if there were error messages with the other crashes and it seems like a few times I got back a 503 from Reddit to which I panic. I'll remove that and see if it helps but I don't think this is the cause every time since it happens very periodically and predictably.

KevinRhyne commented 7 years ago

Please post your console output.

guliyevemil1 commented 7 years ago

Uh... I am really not sure what to tell you. There are no logs when the crash happened except for the 503 that I just told you about.

guliyevemil1 commented 7 years ago

I may have figured it out. I'm going to test my hypothesis and let you know within 24 hours.

guliyevemil1 commented 7 years ago

I think I know what's going on. I don't understand why this is the case but I think the body of the responses aren't necessarily always closed when we make an http request and receive an error. This is why I originally was closing the body of the request before checking for the error in my pull request but when you asked me to change it, I couldn't find in the source code why this could be happening and I didn't want to argue so I complied. However, this is something I have observed to be a problem in the past in other code I've written. I don't understand what causes this.

What led me to conclude this is that before I made my pull request, it was the case that every response body was not being closed which lead to OOM very fast. Afterward, the crashes were still happening but less frequently. Yesterday, I realized that due to a minor bug, I was making a lot of requests that were invalid so I was receiving a lot of error responses. I fixed this bug and my memory usage has become much more stable and has not yet crashed. However, it looks like it'll probably crash within 12 hours because I still receive a lot of 403's and 404's from reddit, and I believe body responses of these messages are not being closed. I'll make the same pull request again. Thanks.

turnage commented 7 years ago

Thank you for submitting your pr! In the future if you believe I'm incorrect please do say so.

Closed in #30