wakatime / delphi-wakatime

Embarcadero Delphi plugin for automatic time tracking and metrics generated from your programming activity.
https://wakatime.com/delphi
BSD 3-Clause "New" or "Revised" License
29 stars 6 forks source link

Memory Leak with Delphi 11.3 #6

Closed mobius1qwe closed 1 year ago

mobius1qwe commented 1 year ago

Describe the bug Internal memory leak with wakatime client invokes. After some hours with the IDE opened it starts invoking a huge ammount of wakatime-cli instances, over 25 per second, which eats all the system's processor and memory, to fix you need to close all delphi instances and open again. I call this "internal leak".

Happens with me on 11.3

To Reproduce Steps to reproduce the behavior:

  1. Install wakatime (ofc)
  2. open a project and work on it for a few hours (6+)
  3. type or save the current file (ctrl+S) or ident-and-save (Ctrl+D+S)
  4. See error

Expected behavior not eat all my CPU would be nice.

Desktop (please complete the following information):

diegomgarcia commented 1 year ago

Thanks for the report @mobius1qwe, I think this is the main cause for the other issue that is crashing the IDE on exit.

I will investigate that with the full logs of FastMM to see if I can track the main issue and solve it. However I'm relocating to another country so I expect to work on this issue and the other ones at the beginning of the next month.

mobius1qwe commented 1 year ago

Upon further usage and investigation, the leak (or slowing) seems to happen after a debugging period, if we use the IDE without running in debug mode the code, it doesn't seem to happen.

MeSHA256 commented 1 year ago

Upon further usage and investigation, the leak (or slowing) seems to happen after a debugging period, if we use the IDE without running in debug mode the code, it doesn't seem to happen.

I have this problem with CPU about once every 1-2 hours, even without using debug mode. But without memory leaks in Delphi 10.4. Both in Delphi 11.3.

diegomgarcia commented 1 year ago

Hey guys,

I've just commited the fix for this issue on the development branch, can you guys test it before I merge with the main?

MeSHA256 commented 1 year ago

Hey guys,

I've just commited the fix for this issue on the development branch, can you guys test it before I merge with the main?

Yeah, no problem.

MeSHA256 commented 1 year ago

Delphi 11.3. No problems after more than 3 hours of coding.

When LSP took the entire CPU for itself, I got it: photo_2023-09-27_17-11-05

But I think the problem here is only in the LSP itself, because I have to kill it regularly, otherwise it is impossible to work. Tomorrow I will test your fix on 10.4.

diegomgarcia commented 1 year ago

Thats a really great news Michael!

Thanks for testing it and for the feedback.

Lets monitor a few days to see how it will behave.

I will just take a look on how the notifier thread is releasing the wakatime-cli.exe as it seems that it has lots of then alocated on your screenshot, maybe I can improve that also.

Delphi 11.3. No problems after more than 3 hours of coding.

When LSP took the entire CPU for itself, I got it: photo_2023-09-27_17-11-05

But I think the problem here is only in the LSP itself, because I have to kill it regularly, otherwise it is impossible to work. Tomorrow I will test your fix on 10.4.

diegomgarcia commented 1 year ago

I've just reviewed the NotifierThread code and it doesn't has any pottential leak on it, however I've fixed a typo that wasn't applying one of the cleanup of the command line strings. In some cases depending on some factors, maybe it could fail to report it to wakatime. Also improved some variable naming ...

So for now I think this version is good to go. I've been using since yesterday without problems, will just wait a couple of days to see if we got any other reports of use here so we can validate it.

MeSHA256 commented 1 year ago

Delphi 10.4. Everything all right. No more problems with CPU overload.

diegomgarcia commented 1 year ago

Awesome news!!!

Thanks Michael!

MeSHA256 commented 1 year ago

Awesome news!!!

Thanks Michael!

Thanks for your implementation of wakatime for Delphi :)

diegomgarcia commented 1 year ago

You are welcome Michael =)

I was needing this to track my projects to report to the customers as we usually do here with C# and other languages. So this was kind of a must have to do job LOL.

Hope everyone like it =)