Issue with 'mg_timer_init()'

[gvanem]

Hello folks.

I use mg_timer_init() like this:

 struct mg_mgr mgr;
 struct mg_timer fixed_timer;
 ...
 mg_timer_init (&mgr.timers, &fixed_timer, 5000, MG_TIMER_ONCE, net_timeout, 0);
 ...
 static void net_timeout (void *fn_data)
 {
   // set a flag to cause an 'exit(1)'
 }

in my program and I build with ASAN (Address Sanitation) on MSVC and clang-cl.

The issue I have is with mg_mgr_free() is calling free(t) for such a timer since I get this ASAN crash at program-exit:

=================================================================
==12056==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x7ff7f74b9178 in thread T0
    #0 0x7ffdbbb923f8 in free D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_malloc_win.cpp:230
    #1 0x7ff7f7209bd1 in mg_mgr_free F:\gv\dx-radio\gv-Dump1090\externals\mongoose.c:4301
    #2 0x7ff7f71f5233 in net_exit F:\gv\dx-radio\gv-Dump1090\net_io.c:1950
    #3 0x7ff7f71e473c in modeS_exit F:\gv\dx-radio\gv-Dump1090\dump1090.c:3031
    #4 0x7ff7f71ebb26 in main F:\gv\dx-radio\gv-Dump1090\dump1090.c:3614
    #5 0x7ff7f73bd5e3 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #6 0x7ff7f73bd5e3 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
...

So how does Mongoose differentiate between user-added timers or timers added by mg_timer_add()?
AFAICS there should be a t->added = 1 internal flag for this case. Thus not causing this illegal free.
Something like:

--- a/externals/mongoose.h
+++ b/externals/mongoose.h
@@ -950,6 +950,7 @@ struct mg_timer {
   uint64_t period_ms;       // Timer period in milliseconds
   uint64_t expire;          // Expiration timestamp in milliseconds
   unsigned flags;           // Possible flags values below
+  unsigned added;           // Createad by 'mg_timer_add()'
 #define MG_TIMER_ONCE 0     // Call function once
 #define MG_TIMER_REPEAT 1   // Call function periodically

--- a/externals/mongoose.c
+++ b/externals/mongoose.c
@@ -4291,6 +4291,7 @@ struct mg_timer *mg_timer_add(struct mg_mgr *mgr, uint64_t milliseconds,
   if (t != NULL) {
     mg_timer_init(&mgr->timers, t, milliseconds, flags, fn, arg);
     t->id = mgr->timerid++;
+    t->added = 1;
   }
   return t;
 }
@@ -4298,7 +4299,13 @@ struct mg_timer *mg_timer_add(struct mg_mgr *mgr, uint64_t milliseconds,
 void mg_mgr_free(struct mg_mgr *mgr) {
   struct mg_connection *c;
   struct mg_timer *tmp, *t = mgr->timers;
-  while (t != NULL) tmp = t->next, free(t), t = tmp;
+  while (t != NULL) {
+    tmp = t->next;
+    if (t->added)
+       free(t);
+     t = tmp;
+  }
+
   mgr->timers = NULL;  // Important. Next call to poll won't touch timers
   for (c = mgr->conns; c != NULL; c = c->next) c->is_closing = 1;

[scaprile]

We should perhaps clarify the documentation.
mg_timer_init() is intended to be called to init a timer that was previously added by mg_timer_add(), as it is the latter who allocs memory for the timer.
If you just need a timer "handy", add it with a very long timeout so it does not fire, and then set init it as you need it when you need it

[gvanem]

If you just need a timer "handy", add it with a very long timeout so it does not fire,

This looks weird. And how to postpone a timer? I now use code like this:

  mg_timer *t = &fixed_timer;
  t->expire = SOME_TIMEOUT_INTO_THE_FUTURE + mg_millis();

But it's not correct with the current mg_timer_*() functions.

[scaprile]

mg_timer_add(,0x7fffffff,)
Then do whatever you need with mg_timer_init()
If you need to postpone it, just re-init, and don't hesitate to use 0x7fffffff again.

I don't think we will implement something different, most (I'd say all) our timer usage works fine with this current API

PS: nothing stops you from mg_timer *t = calloc(); in order to be compatible with the free() call, then you'd just be implementing a modified timer_add() yourself

[gvanem]

OK.