Fixed ticket #939: Throwing exception inside exception handler will cause infinite loop (thanks Roman Puls for the report)

 - exception handler is now popped from the stack immediately in PJ_THROW


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

3 files changed, 169 insertions, 16 deletions

diff --git a/pjlib/include/pj/except.h b/pjlib/include/pj/except.h
index e5a8c899..df9bb461 100644
--- a/pjlib/include/pj/except.h
+++ b/pjlib/include/pj/except.h
@@ -57,17 +57,6 @@ PJ_BEGIN_DECL
  * The exception handling mechanism is completely thread safe, so the exception
  * thrown by one thread will not interfere with other thread.
  *
- * CAVEATS:
- *  - unlike C++ exception, the scheme here won't call destructors of local
- *    objects if exception is thrown. Care must be taken when a function
- *    hold some resorce such as pool or mutex etc.
- *  - You CAN NOT make nested exception in one single function without using
- *    a nested PJ_USE_EXCEPTION.
- *  - You can not provide more than PJ_CATCH or PJ_CATCH_ANY nor use PJ_CATCH
- *    and PJ_CATCH_ANY for a single PJ_TRY.
- *  - Exceptions will always be caught by the first handler (unlike C++ where
- *    exception is only caught if the type matches.
- *
  * The exception handling constructs are similar to C++. The blocks will be
  * constructed similar to the following sample:
  *
@@ -127,6 +116,66 @@ PJ_BEGIN_DECL
  * #PJ_NO_MEMORY_EXCEPTION which is declared in <pj/pool.h>. This exception
  * ID is raised by default pool policy when it fails to allocate memory.
  *
+ * CAVEATS:
+ *  - unlike C++ exception, the scheme here won't call destructors of local
+ *    objects if exception is thrown. Care must be taken when a function
+ *    hold some resorce such as pool or mutex etc.
+ *  - You CAN NOT make nested exception in one single function without using
+ *    a nested PJ_USE_EXCEPTION. Samples:
+  \verbatim
+	void wrong_sample()
+	{
+	    PJ_USE_EXCEPTION;
+
+	    PJ_TRY {
+		// Do stuffs
+		...
+	    }
+	    PJ_CATCH_ANY {
+		// Do other stuffs
+		....
+		..
+
+		// The following block is WRONG! You MUST declare 
+		// PJ_USE_EXCEPTION once again in this block.
+		PJ_TRY {
+		    ..
+		}
+		PJ_CATCH_ANY {
+		    ..
+		}
+		PJ_END;
+	    }
+	    PJ_END;
+	}
+
+  \endverbatim
+
+ *  - You MUST NOT exit the function inside the PJ_TRY block. The correct way
+ *    is to return from the function after PJ_END block is executed. 
+ *    For example, the following code will yield crash not in this code,
+ *    but rather in the subsequent execution of PJ_TRY block:
+  \verbatim
+        void wrong_sample()
+	{
+	    PJ_USE_EXCEPTION;
+
+	    PJ_TRY {
+		// do some stuffs
+		...
+		return;	        <======= DO NOT DO THIS!
+	    }
+	    PJ_CATCH_ANY {
+	    }
+	    PJ_END;
+	}
+  \endverbatim
+  
+ *  - You can not provide more than PJ_CATCH or PJ_CATCH_ANY nor use PJ_CATCH
+ *    and PJ_CATCH_ANY for a single PJ_TRY.
+ *  - Exceptions will always be caught by the first handler (unlike C++ where
+ *    exception is only caught if the type matches.
+
  * \section PJ_EX_KEYWORDS Keywords
  *
  * \subsection PJ_THROW PJ_THROW(expression)
@@ -310,7 +359,7 @@ PJ_DECL(void) pj_push_exception_handler_(struct pj_exception_state_t *rec);
 /**
  * Pop exception handler.
  */
-PJ_DECL(void) pj_pop_exception_handler_(void);
+PJ_DECL(void) pj_pop_exception_handler_(struct pj_exception_state_t *rec);
 
 /**
  * Declare that the function will use exception.
@@ -343,7 +392,7 @@ PJ_DECL(void) pj_pop_exception_handler_(void);
  * End of exception specification block.
  * @hideinitializer
  */
-#define PJ_END			pj_pop_exception_handler_(); \
+#define PJ_END			pj_pop_exception_handler_(&pj_x_except__); \
 			    } else {}
 
 /**
diff --git a/pjlib/src/pj/except.c b/pjlib/src/pj/except.c
index 66aa56e6..61c882c4 100644
--- a/pjlib/src/pj/except.c
+++ b/pjlib/src/pj/except.c
@@ -50,6 +50,7 @@ PJ_DEF(void) pj_throw_exception_(int exception_id)
         pj_assert(handler != NULL);
         /* This will crash the system! */
     }
+    pj_pop_exception_handler_(handler);
     pj_longjmp(handler->state, exception_id);
 }
 
@@ -86,14 +87,15 @@ PJ_DEF(void) pj_push_exception_handler_(struct pj_exception_state_t *rec)
     pj_thread_local_set(thread_local_id, rec);
 }
 
-PJ_DEF(void) pj_pop_exception_handler_(void)
+PJ_DEF(void) pj_pop_exception_handler_(struct pj_exception_state_t *rec)
 {
     struct pj_exception_state_t *handler;
 
     handler = (struct pj_exception_state_t *)
 	      pj_thread_local_get(thread_local_id);
-    pj_assert(handler != NULL);
-    pj_thread_local_set(thread_local_id, handler->prev);
+    if (handler && handler==rec) {
+	pj_thread_local_set(thread_local_id, handler->prev);
+    }
 }
 #endif
 
diff --git a/pjlib/src/pjlib-test/exception.c b/pjlib/src/pjlib-test/exception.c
index 94ab861a..6e141054 100644
--- a/pjlib/src/pjlib-test/exception.c
+++ b/pjlib/src/pjlib-test/exception.c
@@ -63,6 +63,53 @@ static int throw_id_2(void)
     PJ_UNREACHED(return -1;)
 }
 
+static int try_catch_test(void)
+{
+    PJ_USE_EXCEPTION;
+    int rc = -200;
+
+    PJ_TRY {
+	PJ_THROW(ID_1);
+    }
+    PJ_CATCH_ANY {
+	rc = 0;
+    }
+    PJ_END;
+    return rc;
+}
+
+static int throw_in_handler(void)
+{
+    PJ_USE_EXCEPTION;
+    int rc = 0;
+
+    PJ_TRY {
+	PJ_THROW(ID_1);
+    }
+    PJ_CATCH_ANY {
+	if (PJ_GET_EXCEPTION() != ID_1)
+	    rc = -300;
+	else
+	    PJ_THROW(ID_2);
+    }
+    PJ_END;
+    return rc;
+}
+
+static int return_in_handler(void)
+{
+    PJ_USE_EXCEPTION;
+
+    PJ_TRY {
+	PJ_THROW(ID_1);
+    }
+    PJ_CATCH_ANY {
+	return 0;
+    }
+    PJ_END;
+    return -400;
+}
+
 
 static int test(void)
 {
@@ -153,6 +200,61 @@ static int test(void)
     if (rc != 0)
 	return rc;
 
+    /*
+     * Nested handlers
+     */
+    PJ_TRY {
+	rc = try_catch_test();
+    }
+    PJ_CATCH_ANY {
+	rc = -70;
+    }
+    PJ_END;
+
+    if (rc != 0)
+	return rc;
+
+    /*
+     * Throwing exception inside handler
+     */
+    rc = -80;
+    PJ_TRY {
+	int rc2;
+	rc2 = throw_in_handler();
+	if (rc2)
+	    rc = rc2;
+    }
+    PJ_CATCH_ANY {
+	if (PJ_GET_EXCEPTION() == ID_2) {
+	    rc = 0;
+	} else {
+	    rc = -90;
+	}
+    }
+    PJ_END;
+
+    if (rc != 0)
+	return rc;
+
+
+    /*
+     * Return from handler. Returning from the function inside a handler
+     * should be okay (though returning from the function inside the
+     * PJ_TRY block IS NOT OKAY!!). We want to test to see if handler
+     * is cleaned up properly, but not sure how to do this.
+     */
+    PJ_TRY {
+	int rc2;
+	rc2 = return_in_handler();
+	if (rc2)
+	    rc = rc2;
+    }
+    PJ_CATCH_ANY {
+	rc = -100;
+    }
+    PJ_END;
+
+
     return 0;
 }