Merge pull request #759 from taosdata/hotfix/tbase-1217

fix TBASE-1217

src/util/src/ttimer.c

@@ -14,6 +14,7 @@
  */
 
 #include "os.h"
+#include <inttypes.h>
 #include "tlog.h"
 #include "tsched.h"
 #include "ttime.h"
@@ -253,13 +254,13 @@ static void processExpiredTimer(void* handle, void* arg) {
   timer->executedBy = taosGetPthreadId();
   uint8_t state = atomic_val_compare_exchange_8(&timer->state, TIMER_STATE_WAITING, TIMER_STATE_EXPIRED);
   if (state == TIMER_STATE_WAITING) {
-    const char* fmt = "%s timer[id=%lld, fp=%p, param=%p] execution start.";
+    const char* fmt = "%s timer[id=" PRIuPTR ", fp=%p, param=%p] execution start.";
     tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
 
     (*timer->fp)(timer->param, (tmr_h)timer->id);
     atomic_store_8(&timer->state, TIMER_STATE_STOPPED);
 
-    fmt = "%s timer[id=%lld, fp=%p, param=%p] execution end.";
+    fmt = "%s timer[id=" PRIuPTR ", fp=%p, param=%p] execution end.";
     tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
   }
   removeTimer(timer->id);
@@ -267,7 +268,7 @@ static void processExpiredTimer(void* handle, void* arg) {
 }
 
 static void addToExpired(tmr_obj_t* head) {
-  const char* fmt = "%s adding expired timer[id=%lld, fp=%p, param=%p] to queue.";
+  const char* fmt = "%s adding expired timer[id=" PRIuPTR ", fp=%p, param=%p] to queue.";
 
   while (head != NULL) {
     uintptr_t id = head->id;
@@ -281,7 +282,7 @@ static void addToExpired(tmr_obj_t* head) {
     schedMsg.thandle = NULL;
     taosScheduleTask(tmrQhandle, &schedMsg);
 
-    tmrTrace("timer[id=%lld] has been added to queue.", id);
+    tmrTrace("timer[id=" PRIuPTR "] has been added to queue.", id);
     head = next;
   }
 }
@@ -295,7 +296,7 @@ static uintptr_t doStartTimer(tmr_obj_t* timer, TAOS_TMR_CALLBACK fp, int msecon
   timer->ctrl = ctrl;
   addTimer(timer);
 
-  const char* fmt = "%s timer[id=%lld, fp=%p, param=%p] started";
+  const char* fmt = "%s timer[id=" PRIuPTR ", fp=%p, param=%p] started";
   tmrTrace(fmt, ctrl->label, timer->id, timer->fp, timer->param);
 
   if (mseconds == 0) {
@@ -380,40 +381,37 @@ static void taosTimerLoopFunc(int signo) {
 }
 
 static bool doStopTimer(tmr_obj_t* timer, uint8_t state) {
-  bool reusable = false;
-
   if (state == TIMER_STATE_WAITING) {
+    bool reusable = false;
     if (removeFromWheel(timer)) {
       removeTimer(timer->id);
       // only safe to reuse the timer when timer is removed from the wheel.
       // we cannot guarantee the thread safety of the timr in all other cases.
       reusable = true;
     }
-    const char* fmt = "%s timer[id=%lld, fp=%p, param=%p] is cancelled.";
+    const char* fmt = "%s timer[id=" PRIuPTR ", fp=%p, param=%p] is cancelled.";
     tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
-  } else if (state != TIMER_STATE_EXPIRED) {
+    return reusable;
+  }
+  
+  if (state != TIMER_STATE_EXPIRED) {
     // timer already stopped or cancelled, has nothing to do in this case
-  } else if (timer->executedBy == taosGetPthreadId()) {
+    return false;
+  }
+  
+  if (timer->executedBy == taosGetPthreadId()) {
     // taosTmrReset is called in the timer callback, should do nothing in this
     // case to avoid dead lock. note taosTmrReset must be the last statement
     // of the callback funtion, will be a bug otherwise.
-  } else {
-    assert(timer->executedBy != taosGetPthreadId());
-
-    const char* fmt = "%s timer[id=%lld, fp=%p, param=%p] fired, waiting...";
-    tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
-
-    for (int i = 1; atomic_load_8(&timer->state) != TIMER_STATE_STOPPED; i++) {
-      if (i % 1000 == 0) {
-        sched_yield();
-      }
-    }
-
-    fmt = "%s timer[id=%lld, fp=%p, param=%p] stopped.";
-    tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
+    return false;
   }
-
-  return reusable;
+  
+  // timer callback is executing in another thread, we SHOULD wait it to stop,
+  // BUT this may result in dead lock if current thread are holding a lock which
+  // the timer callback need to acquire. so, we HAVE TO return directly.
+  const char* fmt = "%s timer[id=" PRIuPTR ", fp=%p, param=%p] is executing and cannot be stopped.";
+  tmrTrace(fmt, timer->ctrl->label, timer->id, timer->fp, timer->param);
+  return false;
 }
 
 bool taosTmrStop(tmr_h timerId) {
@@ -421,7 +419,7 @@ bool taosTmrStop(tmr_h timerId) {
 
   tmr_obj_t* timer = findTimer(id);
   if (timer == NULL) {
-    tmrTrace("timer[id=%lld] does not exist", id);
+    tmrTrace("timer[id=" PRIuPTR "] does not exist", id);
     return false;
   }
 
@@ -448,7 +446,7 @@ bool taosTmrReset(TAOS_TMR_CALLBACK fp, int mseconds, void* param, void* handle,
   bool       stopped = false;
   tmr_obj_t* timer = findTimer(id);
   if (timer == NULL) {
-    tmrTrace("%s timer[id=%lld] does not exist", ctrl->label, id);
+    tmrTrace("%s timer[id=" PRIuPTR "] does not exist", ctrl->label, id);
   } else {
     uint8_t state = atomic_val_compare_exchange_8(&timer->state, TIMER_STATE_WAITING, TIMER_STATE_CANCELED);
     if (!doStopTimer(timer, state)) {
@@ -463,7 +461,7 @@ bool taosTmrReset(TAOS_TMR_CALLBACK fp, int mseconds, void* param, void* handle,
     return stopped;
   }
 
-  tmrTrace("%s timer[id=%lld] is reused", ctrl->label, timer->id);
+  tmrTrace("%s timer[id=" PRIuPTR "] is reused", ctrl->label, timer->id);
 
   // wait until there's no other reference to this timer,
   // so that we can reuse this timer safely.