fixed concurrency issues

hopefully everything is awesome now, given that the callstack tracker needed to manage each thread's stack independently, and wasn't, but now is.

diff --git a/nucleus/library/application/callstack_tracker.cpp b/nucleus/library/application/callstack_tracker.cpp
index 1c079db0f39ebe1e3a8859912ee03c75152eea2e..1773ce58af39c2f3f6dbcc6d088c53bee40c41f4 100644 (file)
--- a/nucleus/library/application/callstack_tracker.cpp
+++ b/nucleus/library/application/callstack_tracker.cpp
@@ -175,55 +175,77 @@ bool callstack_tracker::update_line(int line)
   return true;
 }
 
+// helpful macro makes sure we stay within our buffer for string storage of the stack trace.
+#define CHECK_SPACE_IN_BUFFER(desired_chunk) \
+  /* being slightly paranoid about the space, but we don't want any buffer overflows. */ \
+  if (space_used_in_buffer + desired_chunk >= full_size_needed - 4) { \
+    printf("callstack_tracker::full_trace: failure in size estimation--we would have blown out of the buffer"); \
+    return to_return; \
+  } else { \
+    space_used_in_buffer += desired_chunk; \
+  }
+  
 char *callstack_tracker::full_trace() const
 {
   auto_synchronizer l(callstack_tracker::__callstack_tracker_synchronizer());
 
   if (_unusable) return strdup("");
-//printf("fulltrace in\n");
-  char *to_return = (char *)malloc(full_trace_size());
+  int full_size_needed = full_trace_size();
+  char *to_return = (char *)malloc(full_size_needed);
+//printf("fulltrace allocated %d bytes for trace.\n", full_size_needed);
   to_return[0] = '\0';
   if (!_depth) {
     strcat(to_return, emptiness_note);
     return to_return;
   }
+
   const int initial_len = MAX_TEXT_FIELD + 8;
   char temp[initial_len];
-  int allowed_len = initial_len;
-    // space provided for one text line.
+  int space_left_in_line;  // the space provided for one text line.
+
+  int space_used_in_buffer = 0;
+    /* tracks whether we're getting close to the buffer limit.  technically, this
+    should not happen, since we calculated the space ahead of time... but it is good
+    to ensure we don't overflow the buffer in case we were not accurate. */
+
   // start at top most active frame and go down towards bottom most.
   for (int i = _depth; i >= 1; i--) {
+    CHECK_SPACE_IN_BUFFER(1);
     strcat(to_return, "\t");  // we left space for this and \n at end.
+    space_left_in_line = initial_len;  // reset our counter per line now.
     temp[0] = '\0';
     int len_class = strlen(_bt->_records[i]._class);
     int len_func = strlen(_bt->_records[i]._func);
-    if (allowed_len > len_class + len_func + 6) {
-      allowed_len -= len_class + len_func + 6;
+    if (space_left_in_line > len_class + len_func + 6) {
+      space_left_in_line -= len_class + len_func + 6;
       sprintf(temp, "\"%s::%s\", ", _bt->_records[i]._class,
           _bt->_records[i]._func);
+      CHECK_SPACE_IN_BUFFER(strlen(temp));
       strcat(to_return, temp);
     }
 
     temp[0] = '\0';
     int len_file = strlen(_bt->_records[i]._file);
-    if (allowed_len > len_file + 4) {
-      allowed_len -= len_file + 4;
+    if (space_left_in_line > len_file + 4) {
+      space_left_in_line -= len_file + 4;
       sprintf(temp, "\"%s\", ", _bt->_records[i]._file);
+      CHECK_SPACE_IN_BUFFER(strlen(temp));
       strcat(to_return, temp);
     }
 
     temp[0] = '\0';
     sprintf(temp, "\"line=%d\"", _bt->_records[i]._line);
     int len_line = strlen(temp);
-    if (allowed_len > len_line) {
-      allowed_len -= len_line;
+    if (space_left_in_line > len_line) {
+      space_left_in_line -= len_line;
+      CHECK_SPACE_IN_BUFFER(strlen(temp));
       strcat(to_return, temp);
     }
 
+    CHECK_SPACE_IN_BUFFER(1);
     strcat(to_return, "\n");  // we left space for this already.
   }
 
-//printf("fulltrace out\n");
   return to_return;
 }
 

diff --git a/nucleus/library/tests_application/test_callstack_tracker.cpp b/nucleus/library/tests_application/test_callstack_tracker.cpp
index 25711a360d041fbf3e63bb0677b7afb4a81a2eb6..825f0c70fffb64f06371b50041406725912fb5f7 100644 (file)
--- a/nucleus/library/tests_application/test_callstack_tracker.cpp
+++ b/nucleus/library/tests_application/test_callstack_tracker.cpp
@@ -58,8 +58,12 @@ private:
   int sub_call_2();
   int sub_call_3();
   int sub_call_4();
+
+  int recursive_factorial(int num);
 };
 
+//////////////
+
 int test_callstack_tracker::run_filestack_simple()
 {
   FUNCDEF("run_filestack_simple")
@@ -70,6 +74,8 @@ int test_callstack_tracker::run_filestack_simple()
   return 0;
 }
 
+//////////////
+
 int test_callstack_tracker::sub_call_1()
 {
   FUNCDEF("sub_call_1")
@@ -112,13 +118,35 @@ int test_callstack_tracker::run_filestack_middling()
 int test_callstack_tracker::run_filestack_complex()
 {
   FUNCDEF("run_filestack_complex")
-  #ifdef ENABLE_CALLSTACK_TRACKING
-//do something recursive and show an elaborate stack
-//return an error if any problem.
-  #endif
+  int factotum = recursive_factorial(37);
+  if (factotum < 0) {
+    // uh-oh, there was an actual failure of some sort.
+    return 1;
+  }
   return 0;
 }
 
+//////////////
+
+int test_callstack_tracker::recursive_factorial(int num)
+{
+  FUNCDEF("recursive_factorial")
+  if (num < 0) {
+    return -1;  // signifies that things have gone badly.
+  }
+  if (num <= 1) {
+    #ifdef ENABLE_CALLSTACK_TRACKING
+      // now show the callstack, since we should be at the deepest level of nesting for this factorial implementation.
+      GET_AND_TEST_STACK_TRACE("trace of recursive_factorial:", -1);
+    #endif
+    return 1;
+  } else {
+    return num * recursive_factorial(num - 1);
+  }
+}
+
+//////////////
+
 int test_callstack_tracker::execute()
 {
   FUNCDEF("execute");