Skip to content

Commit 5e9872e

Browse files
committed
make movePreviousSession idempotent wrt the previous session
1 parent 61d3de2 commit 5e9872e

File tree

2 files changed

+44
-11
lines changed

2 files changed

+44
-11
lines changed

sentry/src/main/java/io/sentry/cache/EnvelopeCache.java

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,10 @@ public void flushPreviousSession() {
462462
public void movePreviousSession(
463463
final @NotNull File currentSessionFile, final @NotNull File previousSessionFile) {
464464
try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) {
465+
if (!currentSessionFile.exists()) {
466+
return;
467+
}
468+
465469
if (previousSessionFile.exists()) {
466470
options.getLogger().log(DEBUG, "Previous session file already exists, deleting it.");
467471
if (!previousSessionFile.delete()) {
@@ -471,19 +475,17 @@ public void movePreviousSession(
471475
}
472476
}
473477

474-
if (currentSessionFile.exists()) {
475-
options.getLogger().log(INFO, "Moving current session to previous session.");
478+
options.getLogger().log(INFO, "Moving current session to previous session.");
476479

477-
try {
478-
final boolean renamed = currentSessionFile.renameTo(previousSessionFile);
479-
if (!renamed) {
480-
options.getLogger().log(WARNING, "Unable to move current session to previous session.");
481-
}
482-
} catch (Throwable e) {
483-
options
484-
.getLogger()
485-
.log(SentryLevel.ERROR, "Error moving current session to previous session.", e);
480+
try {
481+
final boolean renamed = currentSessionFile.renameTo(previousSessionFile);
482+
if (!renamed) {
483+
options.getLogger().log(WARNING, "Unable to move current session to previous session.");
486484
}
485+
} catch (Throwable e) {
486+
options
487+
.getLogger()
488+
.log(SentryLevel.ERROR, "Error moving current session to previous session.", e);
487489
}
488490
}
489491
}

sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,37 @@ class EnvelopeCacheTest {
494494
assertFalse(previousSessionFile.exists())
495495
}
496496

497+
@Test
498+
fun `movePreviousSession is idempotent wrt the previous session`() {
499+
val cache = fixture.getSUT()
500+
501+
val currentSessionFile = EnvelopeCache.getCurrentSessionFile(fixture.options.cacheDirPath!!)
502+
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
503+
504+
// Create a current session file
505+
currentSessionFile.createNewFile()
506+
currentSessionFile.writeText("session content from last run")
507+
508+
assertTrue(currentSessionFile.exists())
509+
assertFalse(previousSessionFile.exists())
510+
511+
// First call: moves session.json -> previous_session.json
512+
cache.movePreviousSession(currentSessionFile, previousSessionFile)
513+
514+
assertFalse(currentSessionFile.exists())
515+
assertTrue(previousSessionFile.exists())
516+
assertEquals("session content from last run", previousSessionFile.readText())
517+
518+
// Second call should be idempotent: previous_session.json should be preserved
519+
// This simulates the race where both MovePreviousSession runnable and
520+
// storeInternal (SessionStart) both call movePreviousSession
521+
cache.movePreviousSession(currentSessionFile, previousSessionFile)
522+
523+
// previous_session.json should still exist with original content
524+
assertTrue(previousSessionFile.exists())
525+
assertEquals("session content from last run", previousSessionFile.readText())
526+
}
527+
497528
@Test
498529
fun `movePreviousSession deletes file and moves session when previous session file already exists`() {
499530
val cache = fixture.getSUT()

0 commit comments

Comments
 (0)