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.


git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@2853 74dad513-b988-da41-8d7b-12977e46ad98

5 files changed, 114 insertions, 2 deletions

diff --git a/pjlib/include/pj/log.h b/pjlib/include/pj/log.h
index 4b2083c2..b7a258f7 100644
--- a/pjlib/include/pj/log.h
+++ b/pjlib/include/pj/log.h
@@ -220,6 +220,10 @@ PJ_DECL(void) pj_log_set_color(int level, pj_color_t color);
  */
 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,6 +292,11 @@ PJ_DECL(pj_color_t) pj_log_get_color(int level);
 #  define pj_log_get_color(level) 0
 
 
+/**
+ * Internal.
+ */
+#   define pj_log_init()	PJ_SUCCESS
+
 #endif	/* #if PJ_LOG_MAX_LEVEL >= 1 */
 
 /** 
diff --git a/pjlib/src/pj/log.c b/pjlib/src/pj/log.c
index 8025b23c..5cb8b040 100644
--- a/pjlib/src/pj/log.c
+++ b/pjlib/src/pj/log.c
@@ -30,6 +30,7 @@ PJ_DEF_DATA(int) pj_log_max_level = PJ_LOG_MAX_LEVEL;
 #else
 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 |
 			    PJ_LOG_HAS_SENDER | PJ_LOG_HAS_NEWLINE |
@@ -66,6 +67,27 @@ static pj_color_t PJ_LOG_COLOR_77 = PJ_TERM_COLOR_R |
 static char log_buffer[PJ_LOG_MAX_SIZE];
 #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)
 {
     log_decor = decor;
@@ -148,6 +170,64 @@ PJ_DEF(pj_log_func*) pj_log_get_log_func(void)
     return log_writer;
 }
 
+/* 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)
 {
@@ -157,13 +237,22 @@ PJ_DEF(void) pj_log( const char *sender, int level,
 #if PJ_LOG_USE_STACK_BUFFER
     char log_buffer[PJ_LOG_MAX_SIZE];
 #endif
-    int len, print_len;
+    int saved_level, len, print_len;
 
     PJ_CHECK_STACK();
 
     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. */
     pj_gettimeofday(&now);
     pj_time_decode(&now, &ptime);
@@ -274,6 +363,11 @@ PJ_DEF(void) pj_log( const char *sender, int level,
 	log_buffer[sizeof(log_buffer)-1] = '\0';
     }
 
+    /* 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);
 }
diff --git a/pjlib/src/pj/os_core_symbian.cpp b/pjlib/src/pj/os_core_symbian.cpp
index 983ce88a..1419fbdc 100644
--- a/pjlib/src/pj/os_core_symbian.cpp
+++ b/pjlib/src/pj/os_core_symbian.cpp
@@ -342,7 +342,10 @@ PJ_DEF(pj_status_t) pj_init(void)
     err = os->Initialize();
     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.
      */ 
diff --git a/pjlib/src/pj/os_core_unix.c b/pjlib/src/pj/os_core_unix.c
index f256ded9..00094cf3 100644
--- a/pjlib/src/pj/os_core_unix.c
+++ b/pjlib/src/pj/os_core_unix.c
@@ -139,6 +139,9 @@ PJ_DEF(pj_status_t) pj_init(void)
 
 #endif
 
+    /* Init logging */
+    pj_log_init();
+
     /* Initialize exception ID for the pool. 
      * Must do so after critical section is configured.
      */
diff --git a/pjlib/src/pj/os_core_win32.c b/pjlib/src/pj/os_core_win32.c
index eac2c502..94e0da4c 100644
--- a/pjlib/src/pj/os_core_win32.c
+++ b/pjlib/src/pj/os_core_win32.c
@@ -152,6 +152,9 @@ PJ_DEF(pj_status_t) pj_init(void)
 	return rc;
     }
     
+    /* Init logging */
+    pj_log_init();
+
     /* Init random seed. */
     /* Or probably not. Let application in charge of this */
     /* pj_srand( GetCurrentProcessId() ); */