perl plugin: Removed commented code
[collectd.git] / src / perl.c
index 7ab45d8..1c0bb4a 100644 (file)
@@ -131,6 +131,7 @@ typedef struct {
 #endif /* COLLECT_DEBUG */
 
        pthread_mutex_t mutex;
+       pthread_mutexattr_t mutexattr;
 } c_ithread_list_t;
 
 /* name / user_data for Perl matches / targets */
@@ -991,30 +992,43 @@ static int pplugin_dispatch_notification (pTHX_ HV *notif)
 } /* static int pplugin_dispatch_notification (HV *) */
 
 /*
- * Call all working functions of the given type.
+ * Call perl sub with thread locking flags handled.
  */
-static int pplugin_call_all (pTHX_ int type, ...)
+static int call_pv_locked (pTHX_ const char* sub_name)
 {
-       int retvals = 0;
-
        _Bool old_running;
-       va_list ap;
-       int ret = 0;
-
-       dSP;
+       int ret;
 
        c_ithread_t *t = (c_ithread_t *)pthread_getspecific(perl_thr_key);
-       if (t == NULL) /* thread destroyed ( c_ithread_destroy*() -> log_debug() ) */
+       if (t == NULL) /* thread destroyed */
                return 0;
 
        old_running = t->running;
        t->running = 1;
-       
+
        if (t->shutdown) {
                t->running = old_running;
                return 0;
        }
 
+       ret = call_pv (sub_name, G_SCALAR);
+
+       t->running = old_running;
+       return ret;
+} /* static int call_pv_locked (pTHX, *sub_name) */
+
+/*
+ * Call all working functions of the given type.
+ */
+static int pplugin_call_all (pTHX_ int type, ...)
+{
+       int retvals = 0;
+
+       va_list ap;
+       int ret = 0;
+
+       dSP;
+
        if ((type < 0) || (type >= PLUGIN_TYPES))
                return -1;
 
@@ -1128,7 +1142,7 @@ static int pplugin_call_all (pTHX_ int type, ...)
 
        PUTBACK;
 
-       retvals = call_pv ("Collectd::plugin_call_all", G_SCALAR);
+       retvals = call_pv_locked (aTHX_ "Collectd::plugin_call_all");
 
        SPAGAIN;
        if (0 < retvals) {
@@ -1141,7 +1155,6 @@ static int pplugin_call_all (pTHX_ int type, ...)
        FREETMPS;
        LEAVE;
 
-       t->running = old_running;
        va_end (ap);
        return ret;
 } /* static int pplugin_call_all (int, ...) */
@@ -1264,7 +1277,6 @@ static int fc_call (pTHX_ int type, int cb_type, pfc_user_data_t *data, ...)
 {
        int retvals = 0;
 
-       _Bool old_running;
        va_list ap;
        int ret = 0;
 
@@ -1273,18 +1285,6 @@ static int fc_call (pTHX_ int type, int cb_type, pfc_user_data_t *data, ...)
 
        dSP;
 
-       c_ithread_t *t = (c_ithread_t *)pthread_getspecific(perl_thr_key);
-       if (t == NULL) /* thread destroyed */
-               return 0;
-
-       old_running = t->running;
-       t->running = 1;
-
-       if (t->shutdown) {
-               t->running = old_running;
-               return 0;
-       }
-
        if ((type < 0) || (type >= FC_TYPES))
                return -1;
 
@@ -1383,7 +1383,7 @@ static int fc_call (pTHX_ int type, int cb_type, pfc_user_data_t *data, ...)
 
        PUTBACK;
 
-       retvals = call_pv ("Collectd::fc_call", G_SCALAR);
+       retvals = call_pv_locked (aTHX_ "Collectd::fc_call");
 
        if ((FC_CB_EXEC == cb_type) && (meta != NULL)) {
                assert (pmeta != NULL);
@@ -1408,7 +1408,6 @@ static int fc_call (pTHX_ int type, int cb_type, pfc_user_data_t *data, ...)
        FREETMPS;
        LEAVE;
 
-       t->running = old_running;
        va_end (ap);
        return ret;
 } /* static int fc_call (int, int, pfc_user_data_t *, ...) */
@@ -1942,7 +1941,6 @@ static int perl_init (void)
        /* Lock the base thread to avoid race conditions with c_ithread_create().
         * See https://github.com/collectd/collectd/issues/9 and
         *     https://github.com/collectd/collectd/issues/1706 for details.
-        * Locking here requires additional check in perl_log() to avoid deadlock.
        */
        assert (aTHX == perl_threads->head->interp);
        pthread_mutex_lock (&perl_threads->mutex);
@@ -2020,7 +2018,6 @@ static void perl_log (int level, const char *msg,
                user_data_t __attribute__((unused)) *user_data)
 {
        dTHX;
-       int locked = 0;
 
        if (NULL == perl_threads)
                return;
@@ -2038,18 +2035,14 @@ static void perl_log (int level, const char *msg,
        /* Lock the base thread if this is not called from one of the read threads
         * to avoid race conditions with c_ithread_create(). See
         * https://github.com/collectd/collectd/issues/9 for details.
-        * Additionally check, if we are called from perl interpreter.
-        * Maybe PTHREAD_MUTEX_RECURSIVE mutex type will be more appropriate?
        */
 
-       if (aTHX == perl_threads->head->interp && !perl_threads->head->running) {
+       if (aTHX == perl_threads->head->interp)
                pthread_mutex_lock (&perl_threads->mutex);
-               locked = 1;
-       }
 
        pplugin_call_all (aTHX_ PLUGIN_LOG, level, msg);
 
-       if (locked)
+       if (aTHX == perl_threads->head->interp)
                pthread_mutex_unlock (&perl_threads->mutex);
 
        return;
@@ -2150,11 +2143,6 @@ static int perl_shutdown (void)
                        nanosleep (&ts_wait, NULL);
                }
                if (thr->running) {
-                       /* This will crash collectd process later due to PERL_SYS_TERM() */
-                       //ERROR ("perl shutdown: thread hangs inside perl. "
-                       //       "Skipped perl interpreter destroy.");
-                       //continue;
-                       
                        ERROR ("perl shutdown: thread hangs inside perl. Thread killed.");
                        pthread_kill (thr->pthread, SIGTERM);
                }
@@ -2163,6 +2151,7 @@ static int perl_shutdown (void)
 
        pthread_mutex_unlock (&perl_threads->mutex);
        pthread_mutex_destroy (&perl_threads->mutex);
+       pthread_mutexattr_destroy (&perl_threads->mutexattr);
 
        sfree (perl_threads);
 
@@ -2302,7 +2291,9 @@ static int init_pi (int argc, char **argv)
        perl_threads = (c_ithread_list_t *)smalloc (sizeof (c_ithread_list_t));
        memset (perl_threads, 0, sizeof (c_ithread_list_t));
 
-       pthread_mutex_init (&perl_threads->mutex, NULL);
+       pthread_mutexattr_init(&perl_threads->mutexattr);
+       pthread_mutexattr_settype(&perl_threads->mutexattr, PTHREAD_MUTEX_RECURSIVE);
+       pthread_mutex_init (&perl_threads->mutex, &perl_threads->mutexattr);
        /* locking the mutex should not be necessary at this point
         * but let's just do it for the sake of completeness */
        pthread_mutex_lock (&perl_threads->mutex);