Changeset 2853

Timestamp:

Aug 5, 2009 10:58:02 AM (15 years ago)

Author:

bennylp

Message:

Ticket #931: Logging function may infinitely recursively calls itself on Windows Mobile (thanks Emil Sturniolo for the report)

• Added feature to temporarily suspend the logging facility while we're in the pj_log() function. The suspension will be thread specific if the platform supports it.

Location:

pjproject/trunk/pjlib

Files:

• include/pj/log.h (modified) (2 diffs)
• src/pj/log.c (modified) (6 diffs)
• src/pj/os_core_symbian.cpp (modified) (1 diff)
• src/pj/os_core_unix.c (modified) (1 diff)
• src/pj/os_core_win32.c (modified) (1 diff)

pjproject/trunk/pjlib/include/pj/log.h

@@ -221 +221 @@
 PJ_DECL(pj_color_t) pj_log_get_color(int level);
 
+/**
+ * Internal function to be called by pj_init()
+ */
+pj_status_t pj_log_init(void);
 
 #else   /* #if PJ_LOG_MAX_LEVEL >= 1 */
@@ -288 +292 @@
 #  define pj_log_get_color(level) 0
 
+
+/**
+ * Internal.
+ */
+#   define pj_log_init()        PJ_SUCCESS
 
 #endif  /* #if PJ_LOG_MAX_LEVEL >= 1 */

pjproject/trunk/pjlib/src/pj/log.c

@@ -31 +31 @@
 static int pj_log_max_level = PJ_LOG_MAX_LEVEL;
 #endif
+static long thread_suspended_tls_id = -1;
 static pj_log_func *log_writer = &pj_log_write;
 static unsigned log_decor = PJ_LOG_HAS_TIME | PJ_LOG_HAS_MICRO_SEC |
@@ -67 +68 @@
 #endif
 
+static void logging_shutdown(void)
+{
+#if PJ_HAS_THREADS
+    if (thread_suspended_tls_id != -1) {
+        pj_thread_local_free(thread_suspended_tls_id);
+        thread_suspended_tls_id = -1;
+    }
+#endif
+}
+
+pj_status_t pj_log_init(void)
+{
+#if PJ_HAS_THREADS
+    if (thread_suspended_tls_id == -1) {
+        pj_thread_local_alloc(&thread_suspended_tls_id);
+        pj_atexit(&logging_shutdown);
+    }
+#endif
+    return PJ_SUCCESS;
+}
+
 PJ_DEF(void) pj_log_set_decor(unsigned decor)
 {
@@ -149 +171 @@
 }
 
+/* Temporarily suspend logging facility for this thread.
+ * If thread local storage/variable is not used or not initialized, then
+ * we can only suspend the logging globally across all threads. This may
+ * happen e.g. when log function is called before PJLIB is fully initialized
+ * or after PJLIB is shutdown.
+ */
+static void suspend_logging(int *saved_level)
+{
+#if PJ_HAS_THREADS
+    if (thread_suspended_tls_id != -1) 
+    {
+        pj_thread_local_set(thread_suspended_tls_id, (void*)PJ_TRUE);
+    } 
+    else
+#endif
+    {
+        pj_log_max_level = 0;
+    }
+    /* Save the level regardless, just in case PJLIB is shutdown
+     * between suspend and resume.
+     */
+    *saved_level = pj_log_max_level;
+}
+
+/* Resume logging facility for this thread */
+static void resume_logging(int *saved_level)
+{
+#if PJ_HAS_THREADS
+    if (thread_suspended_tls_id != -1) 
+    {
+        pj_thread_local_set(thread_suspended_tls_id, (void*)PJ_FALSE);
+    }
+    else
+#endif
+    {
+        /* Only revert the level if application doesn't change the
+         * logging level between suspend and resume.
+         */
+        if (pj_log_max_level==0 && *saved_level)
+            pj_log_max_level = *saved_level;
+    }
+}
+
+/* Is logging facility suspended for this thread? */
+static pj_bool_t is_logging_suspended(void)
+{
+#if PJ_HAS_THREADS
+    if (thread_suspended_tls_id != -1) 
+    {
+        return pj_thread_local_get(thread_suspended_tls_id) != NULL;
+    }
+    else
+#endif
+    {
+        return pj_log_max_level != 0;
+    }
+}
+
 PJ_DEF(void) pj_log( const char *sender, int level, 
                      const char *format, va_list marker)
@@ -158 +238 @@
     char log_buffer[PJ_LOG_MAX_SIZE];
 #endif
-    int len, print_len;
+    int saved_level, len, print_len;
 
     PJ_CHECK_STACK();
@@ -164 +244 @@
     if (level > pj_log_max_level)
         return;
+
+    if (is_logging_suspended())
+        return;
+
+    /* Temporarily disable logging for this thread. Some of PJLIB APIs that
+     * this function calls below will recursively call the logging function 
+     * back, hence it will cause infinite recursive calls if we allow that.
+     */
+    suspend_logging(&saved_level);
 
     /* Get current date/time. */
@@ -275 +364 @@
     }
 
+    /* It should be safe to resume logging at this point. Application can
+     * recursively call the logging function inside the callback.
+     */
+    resume_logging(&saved_level);
+
     if (log_writer)
         (*log_writer)(level, log_buffer, len);

pjproject/trunk/pjlib/src/pj/os_core_symbian.cpp

@@ -343 +343 @@
     if (err != KErrNone)
         return PJ_RETURN_OS_ERROR(err);
-    
+
+    /* Init logging */
+    pj_log_init();
+
     /* Initialize exception ID for the pool. 
      * Must do so after critical section is configured.

pjproject/trunk/pjlib/src/pj/os_core_unix.c

@@ -139 +139 @@
 
 #endif
+
+    /* Init logging */
+    pj_log_init();
 
     /* Initialize exception ID for the pool. 

pjproject/trunk/pjlib/src/pj/os_core_win32.c

@@ -153 +153 @@
     }
     
+    /* Init logging */
+    pj_log_init();
+
     /* Init random seed. */
     /* Or probably not. Let application in charge of this */