Avoid wiping full error context on release

Reviewed-on: #25

CMakeLists.txt

@@ -345,7 +345,14 @@ if(Python3_FOUND)
     # The script configures and drives its own instrumented build tree (under
     # ${CMAKE_BINARY_DIR}/coverage) so this build's binaries and its coverage
     # counters can never be stale or half-instrumented. Reports via gcov.
-    add_custom_target(coverage
+    # Keep the convenient generic name at the top level, but namespace it when
+    # embedded so a parent project can provide its own coverage target.
+    if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+        set(AKERR_COVERAGE_TARGET coverage)
+    else()
+        set(AKERR_COVERAGE_TARGET akerror_coverage)
+    endif()
+    add_custom_target(${AKERR_COVERAGE_TARGET}
         COMMAND ${Python3_EXECUTABLE}
                 ${CMAKE_CURRENT_SOURCE_DIR}/scripts/coverage.py
                 --source-root ${CMAKE_CURRENT_SOURCE_DIR}
@@ -377,7 +384,6 @@ if(Python3_FOUND)
     )
 endif()
 
-set(main_lib_dest "lib/my_library-${MY_LIBRARY_VERSION}")
 install(TARGETS akerror
   EXPORT akerrorTargets
   ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
@@ -401,8 +407,17 @@ configure_package_config_file(
     INSTALL_DESTINATION ${akerror_install_cmakedir}
 )
 
+# The SOVERSION is the project major version, so packages with the same major
+# are ABI-compatible and a different major must be rejected.
+write_basic_package_version_file(
+    "${CMAKE_CURRENT_BINARY_DIR}/akerrorConfigVersion.cmake"
+    VERSION ${PROJECT_VERSION}
+    COMPATIBILITY SameMajorVersion
+)
+
 install(FILES
     "${CMAKE_CURRENT_BINARY_DIR}/akerrorConfig.cmake"
+    "${CMAKE_CURRENT_BINARY_DIR}/akerrorConfigVersion.cmake"
     DESTINATION ${akerror_install_cmakedir}
 )
 

UPGRADING.md

@@ -132,6 +132,11 @@ One recursive lock covers both the pool and the registry, so error
 correctness there is worth more than throughput, but a program that raises
 errors in a hot loop will feel it.
 
+Releasing the last reference to a context remains serialized under that same
+pool lock, but it now resets only the handled/status/reported state, the string
+heads, and the stack-trace cursor. It no longer wipes the whole context buffer,
+so release is a fixed handful of stores rather than a tens-of-kilobytes write.
+
 The per-thread last-ditch context is a whole `akerr_ErrorContext` (tens of
 kilobytes) in thread-local storage, allocated per thread on first use of the
 library from that thread.

src/error.c

@@ -380,10 +380,10 @@ akerr_ErrorContext *akerr_next_error()
 }
 
 /*
- * The wipe returns the slot to the pool, so it and the decrement that triggers
+ * The reset returns the slot to the pool, so it and the decrement that triggers
  * it are one operation under the lock. Otherwise a thread that saw the count
  * reach zero could be handed the slot by akerr_next_error() and start writing
- * its error into it while the releasing thread was still memsetting it.
+ * its error into it while the releasing thread was still resetting it.
  */
 akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err)
 {
@@ -401,7 +401,13 @@ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err)
     }
     if ( err->refcount == 0 ) {
 	oldid = err->arrayid;
-	memset(err, 0x00, sizeof(akerr_ErrorContext));
+	err->handled = false;
+	err->status = 0;
+	err->reported = false;
+	err->message[0] = '\0';
+	err->fname[0] = '\0';
+	err->function[0] = '\0';
+	err->stacktracebuf[0] = '\0';
 	err->stacktracebufptr = (char *)&err->stacktracebuf;
 	err->arrayid = oldid;
 	remaining = NULL;

tests/err_release_clears.c

@@ -1,24 +1,32 @@
 #include "akerror.h"
 #include "err_capture.h"
-#include <string.h>
 
 /*
- * Releasing an error context back to the pool must wipe it, so the next caller
- * that checks it out never sees stale status/message/stacktrace from a previous
- * error. Mutation testing showed the clearing memset in akerr_release_error
- * could be deleted without any test noticing.
+ * Releasing an error context back to the pool must reset the state that affects
+ * the next caller. In particular, a handled error must not make a fresh error
+ * look handled when its slot is recycled.
  */
 
+static int unhandled_calls = 0;
+static int unhandled_status = 0;
+
+static void test_unhandled_handler(akerr_ErrorContext *errctx)
+{
+    unhandled_calls++;
+    unhandled_status = (errctx != NULL) ? errctx->status : 0;
+}
+
 akerr_ErrorContext *boom(void)
 {
     PREPARE_ERROR(e);
-    FAIL_RETURN(e, AKERR_VALUE, "stale dirty message that must not survive");
+    FAIL_RETURN(e, AKERR_VALUE, "first error is handled");
 }
 
 int main(void)
 {
     akerr_capture_install();
     akerr_init();
+    akerr_handler_unhandled_error = &test_unhandled_handler;
 
     /* Raise and fully handle an error; FINISH_NORETURN releases it to the pool. */
     PREPARE_ERROR(e);
@@ -32,13 +40,31 @@ int main(void)
 
     AKERR_CHECK(e == NULL);
 
-    /* The next context handed out is the slot we just released: it must be clean. */
+    /* A fresh error in the recycled slot must not inherit handled=true. */
+    PREPARE_ERROR(fresh);
+    ATTEMPT {
+	CATCH(fresh, boom());
+    } CLEANUP {
+    } PROCESS(fresh) {
+    } FINISH_NORETURN(fresh);
+
+    AKERR_CHECK(unhandled_calls == 1);
+    AKERR_CHECK(unhandled_status == AKERR_VALUE);
+    AKERR_CHECK(fresh == NULL);
+
+    /* The next context handed out is the same slot, with recycle state reset. */
     akerr_ErrorContext *slot = akerr_next_error();
     AKERR_CHECK(slot != NULL);
+    AKERR_CHECK(slot->handled == false);
     AKERR_CHECK(slot->status == 0);
+    AKERR_CHECK(slot->reported == false);
     AKERR_CHECK(slot->message[0] == '\0');
+    AKERR_CHECK(slot->fname[0] == '\0');
+    AKERR_CHECK(slot->function[0] == '\0');
     AKERR_CHECK(slot->stacktracebuf[0] == '\0');
-    AKERR_CHECK(strstr(slot->message, "stale dirty message") == NULL);
+    AKERR_CHECK(slot->stacktracebufptr == (char *)&slot->stacktracebuf);
+    RELEASE_ERROR(slot);
+    AKERR_CHECK(akerr_slots_in_use() == 0);
 
     fprintf(stderr, "err_release_clears ok\n");
     return 0;