Inverted / tasbot_eyes

Software, to control the eyes of the new TASBot, build by blastermak
BSD 2-Clause "Simplified" License
0 stars 3 forks source link

Memory leak #8

Closed R3tr0BoiDX closed 1 year ago

R3tr0BoiDX commented 1 year ago

There's a teeny tiny memory leak. No idea where.

R3tr0BoiDX commented 1 year ago

valgrind reports 27,268 (24 direct, 27,244 indirect) bytes in 1 blocks are definitely lost in loss record 16 of 16 for gif.c:readAnimation() when animation = malloc(sizeof(Animation)); is called. See here.

However, tasbot.c:freeAnimation() should free all of the animation structure? See here.

@compucat, if you eventually have a sec, can I maybe get you to please look at that?

Inverted commented 1 year ago

Can't seem to replicate this specific Valgrind leak. I don't have a Pi with an LED board connected, but I assume this would still happen without that. Could you provide more details on how to replicate it?

R3tr0BoiDX commented 1 year ago

In fairness, I must also confess, I got that report from valgrind when I tested it on my PC. Might be a wise idea to maybe also let run valgrind on the actual Pi eventually. Anyway, I used the valgrind integration of CLion and let the program run for a while, maybe 15 min? I cant quite recall. Ill try to reproduce it myself and let you know (and also, what valgrind reports when I run it on the actual Pi).

Inverted commented 1 year ago

How do you have the program exit cleanly when it's running the loop? Valgrind gives false positives when I force-close it with Ctrl+C. Modifying the main loop to only run an x amount of times only gives a warning about the thread started in network.c, which makes sense given that it's blocking & reading a socket so it can't cleanly exit.

R3tr0BoiDX commented 1 year ago

Oii, that could explain it then, why you arent able to reproduce it then. I just stop running the program within CLion and iirc that just sends an SIGINT to the program. Ill look into it tomorrow! Thx for the heads up, I was unaware of that fact!

R3tr0BoiDX commented 1 year ago

Hey, sorry @Inverted for the delay, there was some uni stuff I needed to get done. I tried running the program with valgrind memcheck on the Pi now, however, I have a slight issue. The eye software needs superuser privileges to run, else its not able to communicate with the LEDs. Err, how do I do that?

I tried (sudo on valgrind)

sudo valgrind --tool=memcheck --xml=yes --xml-file=./tasbot-memcheck.xml  --gen-suppressions=all --leak-check=full --leak-resolution=high --track-origins=yes --vgdb=no ./tasbot_eyes -X -d 18 -c -s 2 

as well as (sudo on ./tasbot_eyes)

valgrind --tool=memcheck --xml=yes --xml-file=./tasbot-memcheck.xml  --gen-suppressions=all --leak-check=full --leak-resolution=high --track-origins=yes --vgdb=no sudo ./tasbot_eyes -X -d 18 -c -s 2 

both with no success.

I also tried to login as superuser with sudo su and then run (no sudo at all)

valgrind --tool=memcheck --xml=yes --xml-file=./tasbot-memcheck.xml  --gen-suppressions=all --leak-check=full --leak-resolution=high --track-origins=yes --vgdb=no ./tasbot_eyes -X -d 18 -c -s 2

which also failed. (Either it can't handle system call 407 or we missing permission)

Inverted commented 1 year ago

Hmm I'm not sure why none of those worked. Valgrind should just work on a Pi, and running it as superuser should have all the permissions you would ever need I'd assume. Do you get any specific error messages/behaviour?

R3tr0BoiDX commented 1 year ago

Ill try again this weekend. From what I remember, when it was saying it couldt handle system call 407 (which is clock_nanosleep_time64, so the 64 bit variant of clock_nanosleep I guess), there was a constant logging of that issue. The one time I ran it in superuser mode, the LEDs did something, but it was veeeeery glitchy. So valgrind and the LED lib also dont seem to work to well together...

What Im also gonna try is, Ill let it run over the course of the next week. From what I remember, dwango wants to let it run for a long period on MAGFest, so when it runs for 4 days (while I'm gone), it should be fine for the event. As you and I both already did changes to code that allocates memory, chances are we already got it fixed. As said, the memory leak is also veeeery smol.

R3tr0BoiDX commented 1 year ago

software was now running for 48+ hours. within the first 24 hours it grew from 4 mb to 11 mb. another 24 hours later it had the exact same memory usage. i ont understand what caused it to inflate, but as it is now, I frankly dont care