[0.17] fix!: align last-crash detection with other SDKs

CHANGELOG.md

@@ -2,10 +2,18 @@
 
 ## Unreleased
 
+**Breaking / Important behavior changes**:
+
+- `sentry_init()` now consumes `<db>/last_crash` after caching its value, aligning crashed-last-run behavior with other Sentry SDKs. ([#2023](https://github.com/getsentry/sentry-native/pull/2023))
+
 **Features**:
 
 - Add `sentry_event_set_level` for setting the level of an individual event. ([#2038](https://github.com/getsentry/sentry-native/pull/2038))
 
+**Deprecations**:
+
+- Deprecate `sentry_clear_crashed_last_run()` because `sentry_init()` now consumes the marker automatically. ([#2023](https://github.com/getsentry/sentry-native/pull/2023))
+
 ## 0.16.5
 
 **Important behavior changes**:

include/sentry.h

@@ -4321,7 +4321,7 @@ SENTRY_API void sentry_transaction_iter_headers(sentry_transaction_t *tx,
  *
  * Notes:
  *   * The underlying value is set by sentry_init() - it must be called first.
- *   * Call sentry_clear_crashed_last_run() to reset for the next app run.
+ *   * sentry_init() clears the persisted value for the next run.
  *
  * Possible return values:
  *   1 = the last run was a crash
@@ -4331,9 +4331,7 @@ SENTRY_API void sentry_transaction_iter_headers(sentry_transaction_t *tx,
 SENTRY_EXPERIMENTAL_API int sentry_get_crashed_last_run(void);
 
 /**
- * Clear the status of the "crashed-last-run". You should explicitly call
- * this after sentry_init() if you're using sentry_get_crashed_last_run().
- * Otherwise, the same information is reported on any subsequent runs.
+ * Clear the persisted status of the "crashed-last-run".
  *
  * Notes:
  *   * This doesn't change the value of sentry_get_crashed_last_run() yet.
@@ -4342,6 +4340,7 @@ SENTRY_EXPERIMENTAL_API int sentry_get_crashed_last_run(void);
  *
  * Returns 0 on success, 1 on error.
  */
+SENTRY_DEPRECATED("The crash marker is cleared by `sentry_init()`.")
 SENTRY_EXPERIMENTAL_API int sentry_clear_crashed_last_run(void);
 
 /**

ndk/lib/src/main/jni/sentry.c

@@ -377,6 +377,9 @@ send_envelope(sentry_envelope_t *envelope, void *data)
 // sentry_backend.h
 extern void sentry__backend_preload(void);
 
+// sentry_database.h
+extern void sentry__retain_crash_marker(sentry_options_t *options);
+
 JNIEXPORT void JNICALL
 Java_io_sentry_ndk_SentryNdk_preloadSentryNative(JNIEnv *env, jclass cls)
 {
@@ -436,6 +439,10 @@ Java_io_sentry_ndk_SentryNdk_initSentryNative(
     options = sentry_options_new();
     ENSURE_OR_FAIL(options);
 
+    // Android SDK needs to retain the marker timestamp for session finalization
+    // and cleans it up when appropriate
+    sentry__retain_crash_marker(options);
+
     // session tracking is enabled by default, but the Android SDK already
     // handles it
     sentry_options_set_auto_session_tracking(options, 0);

src/sentry_core.c

@@ -271,6 +271,9 @@ sentry_init(sentry_options_t *options)
     }
 
     g_last_crash = sentry__has_crash_marker(options);
+    if (g_last_crash && !options->retain_crash_marker) {
+        sentry__clear_crash_marker(options);
+    }
     g_options = options;
 
     // *after* setting the global options, trigger a scope and consent flush,

src/sentry_database.c

@@ -1172,6 +1172,12 @@ sentry__has_crash_marker(const sentry_options_t *options)
     return result;
 }
 
+void
+sentry__retain_crash_marker(sentry_options_t *options)
+{
+    options->retain_crash_marker = true;
+}
+
 bool
 sentry__clear_crash_marker(const sentry_options_t *options)
 {

src/sentry_database.h

@@ -220,6 +220,13 @@ bool sentry__write_crash_marker(const sentry_options_t *options);
  */
 bool sentry__has_crash_marker(const sentry_options_t *options);
 
+/**
+ * Prevents sentry_init() from clearing `<database>/last_crash`.
+ *
+ * Exported for the Android NDK integration.
+ */
+SENTRY_API void sentry__retain_crash_marker(sentry_options_t *options);
+
 /**
  * This will remove the `<database>/last_crash` file.
  */

src/sentry_options.h

@@ -102,6 +102,7 @@ struct sentry_options_s {
     sentry_session_t *session;
     sentry_integration_t **integrations;
     size_t num_integrations;
+    bool retain_crash_marker;
 
     long refcount;
     uint64_t shutdown_timeout;

tests/assertions.py

@@ -477,14 +477,17 @@ def assert_native_crash(envelope, exception_code=None):
 
 
 def assert_crash_timestamp(has_files, tmp_path):
-    # The crash file should survive a `sentry_init` and should still be there
-    # even after restarts.
     if has_files:
         with open("{}/.sentry-native/last_crash".format(tmp_path)) as f:
             crash_timestamp = f.read()
         assert_timestamp(crash_timestamp)
 
 
+def assert_no_crash_timestamp(has_files, tmp_path):
+    if has_files:
+        assert not (Path(tmp_path) / ".sentry-native" / "last_crash").exists()
+
+
 def assert_before_send(envelope):
     event = envelope.get_event()
     assert_matches(event, {"adapted_by": "before_send"})

tests/test_integration_crashpad.py

@@ -21,7 +21,7 @@
     lib_name,
     REPLAY_ID,
 )
-from .conditions import has_crashpad, has_oom
+from .conditions import has_crashpad, has_files, has_oom
 from .proxy import (
     setup_proxy_env_vars,
     cleanup_proxy_env_vars,
@@ -30,6 +30,8 @@
 )
 from .assertions import (
     assert_breadcrumb,
+    assert_crash_timestamp,
+    assert_no_crash_timestamp,
     assert_crashpad_upload,
     assert_meta,
     assert_minidump,
@@ -100,6 +102,9 @@ def test_crashpad_on_crashed_last_run(cmake):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+    # no first-chance handler nor crash marker on macOS
+    if sys.platform != "darwin":
+        assert_crash_timestamp(has_files, tmp_path)
 
     assert not list((tmp_path / ".sentry-native").glob("*.run/*.crash"))
 
@@ -110,6 +115,9 @@ def test_crashpad_on_crashed_last_run(cmake):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+    # no first-chance handler nor crash marker on macOS
+    if sys.platform != "darwin":
+        assert_no_crash_timestamp(has_files, tmp_path)
     callbacks = [
         line
         for line in restarted.stdout.splitlines()

tests/test_integration_http.py

@@ -33,6 +33,8 @@
     assert_user_report,
     assert_minidump,
     assert_breakpad_crash,
+    assert_crash_timestamp,
+    assert_no_crash_timestamp,
     assert_gzip_content_encoding,
     assert_gzip_file_header,
     assert_attachment_view_hierarchy,
@@ -1078,6 +1080,7 @@ def test_on_crashed_last_run(cmake, backend):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+    assert_crash_timestamp(has_files, tmp_path)
 
     run_dirs = list((tmp_path / ".sentry-native").glob("*.run"))
     assert len(run_dirs) == 1
@@ -1092,6 +1095,7 @@ def test_on_crashed_last_run(cmake, backend):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+    assert_no_crash_timestamp(has_files, tmp_path)
     callbacks = [
         line
         for line in restarted.stdout.splitlines()

tests/test_integration_native.py

@@ -28,6 +28,8 @@
 )
 from .assertions import (
     assert_breadcrumb,
+    assert_crash_timestamp,
+    assert_no_crash_timestamp,
     assert_debug_meta_images_do_not_overlap,
     assert_meta,
     assert_native_crash,
@@ -38,6 +40,7 @@
     wait_for_file,
     assert_user_feedback,
 )
+from .conditions import has_files
 from .conditions import has_native, has_oom, is_asan, is_tsan, is_qemu, is_wine
 
 pytestmark = pytest.mark.skipif(
@@ -97,6 +100,7 @@ def test_native_on_crashed_last_run(cmake, httpserver):
     crash_envelope = Envelope.deserialize(httpserver.log[0][0].get_data())
     assert_native_crash(crash_envelope)
     event_id = crash_envelope.headers["event_id"]
+    assert_crash_timestamp(has_files, tmp_path)
 
     db_dir = tmp_path / ".sentry-native"
     run_dirs = list(db_dir.glob("*.run"))
@@ -114,6 +118,7 @@ def test_native_on_crashed_last_run(cmake, httpserver):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+    assert_no_crash_timestamp(has_files, tmp_path)
     callbacks = [
         line
         for line in restarted.stdout.splitlines()

tests/test_integration_stdout.py

@@ -17,7 +17,7 @@
     assert_minidump,
     assert_before_send,
     assert_no_before_send,
-    assert_crash_timestamp,
+    assert_no_crash_timestamp,
     assert_breakpad_crash,
     assert_exception,
     wait_for,
@@ -174,7 +174,7 @@ def test_inproc_crash_stdout(cmake):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="inproc")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -205,7 +205,7 @@ def test_abort_stdout(cmake, backend):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration=backend)
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -224,7 +224,7 @@ def test_inproc_crash_stdout_before_send(cmake):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="inproc")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -239,7 +239,7 @@ def test_inproc_crash_stdout_discarding_on_crash(cmake):
     # since the on_crash() handler discards further processing we expect an empty response
     assert len(output) == 0
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
 
 
 def test_inproc_crash_stdout_before_send_and_on_crash(cmake):
@@ -252,7 +252,7 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake):
     # but we expect no event modification from before_send() since setting on_crash() disables before_send()
     assert_no_before_send(envelope)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="inproc")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -284,7 +284,7 @@ def test_inproc_stack_overflow_stdout(cmake, stack_size):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="inproc")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -297,7 +297,7 @@ def test_breakpad_crash_stdout(cmake):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="breakpad")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -311,7 +311,7 @@ def test_breakpad_crash_stdout_before_send(cmake):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="breakpad")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -327,7 +327,7 @@ def test_breakpad_crash_stdout_discarding_on_crash(cmake):
     # since the on_crash() handler discards further processing we expect an empty response
     assert len(output) == 0
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
 
 
 @pytest.mark.skipif(not has_breakpad or is_qemu, reason="test needs breakpad backend")
@@ -341,7 +341,7 @@ def test_breakpad_crash_stdout_before_send_and_on_crash(cmake):
     # but we expect no event modification from before_send() since setting on_crash() disables before_send()
     assert_no_before_send(envelope)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="breakpad")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)
@@ -380,7 +380,7 @@ def test_breakpad_stack_overflow_stdout(cmake, stack_size):
 
     envelope = Envelope.deserialize(output)
 
-    assert_crash_timestamp(has_files, tmp_path)
+    assert_no_crash_timestamp(has_files, tmp_path)
     assert_meta(envelope, integration="breakpad")
     assert_breadcrumb(envelope)
     assert_attachment(envelope)

tests/unit/test_basic.c

@@ -195,7 +195,8 @@ SENTRY_TEST(crash_marker)
 SENTRY_TEST(crashed_last_run)
 {
     // fails before init() is called
-    TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 1);
+    SENTRY_TEST_DEPRECATED(
+        TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 1));
 
     // clear any leftover from previous test runs
     {
@@ -229,16 +230,33 @@ SENTRY_TEST(crashed_last_run)
         TEST_CHECK_INT_EQUAL(sentry_init(options), 0);
 
         TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1);
-
-        // clear the status and re-init
-        TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 0);
+        TEST_CHECK(!sentry__has_crash_marker(options));
 
         sentry_close();
 
         // no change yet before sentry_init() is called
         TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1);
     }
 
+    {
+        SENTRY_TEST_OPTIONS_NEW(options);
+        sentry_options_set_dsn_n(options, dsn, sizeof(dsn));
+
+        // simulate a crash
+        TEST_CHECK(sentry__write_crash_marker(options));
+
+        sentry__retain_crash_marker(options);
+        TEST_CHECK_INT_EQUAL(sentry_init(options), 0);
+
+        TEST_CHECK_INT_EQUAL(sentry_get_crashed_last_run(), 1);
+        TEST_CHECK(sentry__has_crash_marker(options));
+        // explicit clearing remains supported on all platforms
+        SENTRY_TEST_DEPRECATED(
+            TEST_CHECK_INT_EQUAL(sentry_clear_crashed_last_run(), 0));
+
+        sentry_close();
+    }
+
     {
         SENTRY_TEST_OPTIONS_NEW(options);
         sentry_options_set_dsn_n(options, dsn, sizeof(dsn));