cesanta / mongoose

Embedded Web Server
https://mongoose.ws
Other
10.78k stars 2.68k forks source link

Potential Memory Leak in `mg_timer_free` Function #2768

Open keremzorer opened 1 month ago

keremzorer commented 1 month ago

Description

I have encountered a potential memory leak issue in the Mongoose library related to the mg_timer_free function. This function is intended to free timers, but it appears to leave behind some memory blocks, leading to a memory leak.

Function Definition

void mg_timer_free(struct mg_timer **head, struct mg_timer *t) {
  while (*head && *head != t) head = &(*head)->next;
  if (*head) *head = t->next;
}

Issue Details

In my application, I observe that memory usage increases over time when timers are frequently created and freed using the mg_timer_free function. The current implementation removes the specified timer "t" from the linked list but does not explicitly free the memory allocated for the removed timer, which seems to result in a memory leak.

Question

I appreciate any insights or recommendations you can provide regarding this issue. Thank you for your assistance!

Best Regards, Kerem

scaprile commented 1 month ago

In our tutorials and examples https://mongoose.ws/documentation/#tutorials-and-examples we don't free timers. Is there an actual need to free them ? What is your use case ? Typical usage is to have a handful of timers and restart them, even keeping them restarted, not freeing them, using the RUN_ONCE flag. When you need it again, you init it (don't add, keep its handler and init it) Please explain your use case scenario and your need to free timers. Thank you.

keremzorer commented 1 month ago

My use case involves managing 10 different connections, each with its own timer. When a disconnection occurs, I need to free the associated timers. This is important because maintaining timers for inactive connections can lead to unnecessary resource consumption and potential memory leaks. By freeing the timers upon disconnection, I ensure efficient resource management and system stability. Although typical usage scenarios may involve a small number of timers that are restarted as needed, my specific requirement necessitates the creation and disposal of multiple timers dynamically, making it crucial to free them when they are no longer needed.

scaprile commented 1 month ago

Well, I´d rather not go alloc'ing and freeing in the first place but as Mongoose uses dynamic memory allocation there's nothing we can do about it. Your issue triggered a long delayed internal discussion about our timer API, so let's see what we come up with. In the embedded (microcontroller) world, the usual action is to have a fixed set of timers. I understand you are handling a set of timers on a per-connection basis, so unless you can have a fixed number of connections maximum, you have a strong point to be able to free your timers. In the mean time, if should be safe for you to free yourself the alloced memory after you call timer_free

keremzorer commented 1 month ago

Thank you for your interest and support!

scaprile commented 1 month ago

BTW, there are other ways to implement per connection timers. https://mongoose.ws/documentation/tutorials/error-handling/#handling-connection-timeouts

keremzorer commented 1 month ago

I am going to check it out, thank you for your suggestion, Best Regards, Kerem