Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential Memory Leak in mg_timer_free Function #2768

Open
kzorer opened this issue May 29, 2024 · 6 comments
Open

Potential Memory Leak in mg_timer_free Function #2768

kzorer opened this issue May 29, 2024 · 6 comments
Assignees

Comments

@kzorer
Copy link

kzorer commented May 29, 2024

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

  • Should I call to free(t) after it is removed from the linked list to prevent memory leaks?
  • Could there be any side effects or potential issues with modifying the function as follows?
void mg_timer_free(struct mg_timer **head, struct mg_timer *t) {
  while (*head && *head != t) head = &(*head)->next;
  if (*head) *head = t->next;
  free(t);  // Free the memory allocated for the timer
}

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

Best Regards,
Kerem

@scaprile
Copy link
Collaborator

scaprile commented May 29, 2024

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.

@kzorer
Copy link
Author

kzorer commented May 29, 2024

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
Copy link
Collaborator

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

@kzorer
Copy link
Author

kzorer commented May 30, 2024

Thank you for your interest and support!

@scaprile
Copy link
Collaborator

scaprile commented May 30, 2024

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

@kzorer
Copy link
Author

kzorer commented May 31, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants