diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 5ccfb4fe..3f8a7871 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -8,6 +8,8 @@ #include "CrossPointSettings.h" #include "CrossPointState.h" #include "EpubReaderChapterSelectionActivity.h" +#include "KOReaderCredentialStore.h" +#include "KOReaderSyncActivity.h" #include "MappedInputManager.h" #include "RecentBooksStore.h" #include "ScreenComponents.h" @@ -120,10 +122,49 @@ void EpubReaderActivity::loop() { // Pass input responsibility to sub activity if exists if (subActivity) { subActivity->loop(); + // Deferred exit: process after subActivity->loop() returns to avoid use-after-free + if (pendingSubactivityExit) { + pendingSubactivityExit = false; + exitActivity(); + updateRequired = true; + skipNextButtonCheck = true; // Skip button processing to ignore stale events + } + // Deferred go home: process after subActivity->loop() returns to avoid race condition + if (pendingGoHome) { + pendingGoHome = false; + exitActivity(); + if (onGoHome) { + onGoHome(); + } + return; // Don't access 'this' after callback + } return; } - // Enter chapter selection activity + // Handle pending go home when no subactivity (e.g., from long press back) + if (pendingGoHome) { + pendingGoHome = false; + if (onGoHome) { + onGoHome(); + } + return; // Don't access 'this' after callback + } + + // Skip button processing after returning from subactivity + // This prevents stale button release events from triggering actions + // We wait until: (1) all relevant buttons are released, AND (2) wasReleased events have been cleared + if (skipNextButtonCheck) { + const bool confirmCleared = !mappedInput.isPressed(MappedInputManager::Button::Confirm) && + !mappedInput.wasReleased(MappedInputManager::Button::Confirm); + const bool backCleared = !mappedInput.isPressed(MappedInputManager::Button::Back) && + !mappedInput.wasReleased(MappedInputManager::Button::Back); + if (confirmCleared && backCleared) { + skipNextButtonCheck = false; + } + return; + } + + // Confirm button opens menu if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { // Don't start activity transition while rendering xSemaphoreTake(renderingMutex, portMAX_DELAY); @@ -269,11 +310,8 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction break; } case EpubReaderMenuActivity::MenuAction::GO_HOME: { - // 2. Trigger the reader's "Go Home" callback - if (onGoHome) { - onGoHome(); - } - + // Defer go home to avoid race condition with display task + pendingGoHome = true; break; } case EpubReaderMenuActivity::MenuAction::DELETE_CACHE: { @@ -294,10 +332,34 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction saveProgress(backupSpine, backupPage, backupPageCount); } - exitActivity(); - updateRequired = true; xSemaphoreGive(renderingMutex); - if (onGoHome) onGoHome(); + // Defer go home to avoid race condition with display task + pendingGoHome = true; + break; + } + case EpubReaderMenuActivity::MenuAction::SYNC: { + if (KOREADER_STORE.hasCredentials()) { + xSemaphoreTake(renderingMutex, portMAX_DELAY); + const int currentPage = section ? section->currentPage : 0; + const int totalPages = section ? section->pageCount : 0; + exitActivity(); + enterNewActivity(new KOReaderSyncActivity( + renderer, mappedInput, epub, epub->getPath(), currentSpineIndex, currentPage, totalPages, + [this]() { + // On cancel - defer exit to avoid use-after-free + pendingSubactivityExit = true; + }, + [this](int newSpineIndex, int newPage) { + // On sync complete - update position and defer exit + if (currentSpineIndex != newSpineIndex || (section && section->currentPage != newPage)) { + currentSpineIndex = newSpineIndex; + nextPageNumber = newPage; + section.reset(); + } + pendingSubactivityExit = true; + })); + xSemaphoreGive(renderingMutex); + } break; } } diff --git a/src/activities/reader/EpubReaderActivity.h b/src/activities/reader/EpubReaderActivity.h index ca7c0dc9..0492d89e 100644 --- a/src/activities/reader/EpubReaderActivity.h +++ b/src/activities/reader/EpubReaderActivity.h @@ -19,6 +19,9 @@ class EpubReaderActivity final : public ActivityWithSubactivity { int cachedSpineIndex = 0; int cachedChapterTotalPageCount = 0; bool updateRequired = false; + bool pendingSubactivityExit = false; // Defer subactivity exit to avoid use-after-free + bool pendingGoHome = false; // Defer go home to avoid race condition with display task + bool skipNextButtonCheck = false; // Skip button processing for one frame after subactivity exit const std::function onGoBack; const std::function onGoHome; diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp index a77b37d0..2a66be9f 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.cpp +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.cpp @@ -2,8 +2,6 @@ #include -#include "KOReaderCredentialStore.h" -#include "KOReaderSyncActivity.h" #include "MappedInputManager.h" #include "fontIds.h" @@ -12,25 +10,7 @@ namespace { constexpr int SKIP_PAGE_MS = 700; } // namespace -bool EpubReaderChapterSelectionActivity::hasSyncOption() const { return KOREADER_STORE.hasCredentials(); } - -int EpubReaderChapterSelectionActivity::getTotalItems() const { - // Add 2 for sync options (top and bottom) if credentials are configured - const int syncCount = hasSyncOption() ? 2 : 0; - return epub->getTocItemsCount() + syncCount; -} - -bool EpubReaderChapterSelectionActivity::isSyncItem(int index) const { - if (!hasSyncOption()) return false; - // First item and last item are sync options - return index == 0 || index == getTotalItems() - 1; -} - -int EpubReaderChapterSelectionActivity::tocIndexFromItemIndex(int itemIndex) const { - // Account for the sync option at the top - const int offset = hasSyncOption() ? 1 : 0; - return itemIndex - offset; -} +int EpubReaderChapterSelectionActivity::getTotalItems() const { return epub->getTocItemsCount(); } int EpubReaderChapterSelectionActivity::getPageItems() const { // Layout constants used in renderScreen @@ -64,13 +44,10 @@ void EpubReaderChapterSelectionActivity::onEnter() { renderingMutex = xSemaphoreCreateMutex(); - // Account for sync option offset when finding current TOC index - const int syncOffset = hasSyncOption() ? 1 : 0; selectorIndex = epub->getTocIndexForSpineIndex(currentSpineIndex); if (selectorIndex == -1) { selectorIndex = 0; } - selectorIndex += syncOffset; // Offset for top sync option // Trigger first update updateRequired = true; @@ -95,24 +72,6 @@ void EpubReaderChapterSelectionActivity::onExit() { renderingMutex = nullptr; } -void EpubReaderChapterSelectionActivity::launchSyncActivity() { - xSemaphoreTake(renderingMutex, portMAX_DELAY); - exitActivity(); - enterNewActivity(new KOReaderSyncActivity( - renderer, mappedInput, epub, epubPath, currentSpineIndex, currentPage, totalPagesInSpine, - [this]() { - // On cancel - exitActivity(); - updateRequired = true; - }, - [this](int newSpineIndex, int newPage) { - // On sync complete - exitActivity(); - onSyncPosition(newSpineIndex, newPage); - })); - xSemaphoreGive(renderingMutex); -} - void EpubReaderChapterSelectionActivity::loop() { if (subActivity) { subActivity->loop(); @@ -129,15 +88,7 @@ void EpubReaderChapterSelectionActivity::loop() { const int totalItems = getTotalItems(); if (mappedInput.wasReleased(MappedInputManager::Button::Confirm)) { - // Check if sync option is selected (first or last item) - if (isSyncItem(selectorIndex)) { - launchSyncActivity(); - return; - } - - // Get TOC index (account for top sync offset) - const int tocIndex = tocIndexFromItemIndex(selectorIndex); - const auto newSpineIndex = epub->getSpineIndexForTocIndex(tocIndex); + const auto newSpineIndex = epub->getSpineIndexForTocIndex(selectorIndex); if (newSpineIndex == -1) { onGoBack(); } else { @@ -192,18 +143,13 @@ void EpubReaderChapterSelectionActivity::renderScreen() { const int displayY = 60 + i * 30; const bool isSelected = (itemIndex == selectorIndex); - if (isSyncItem(itemIndex)) { - renderer.drawText(UI_10_FONT_ID, 20, displayY, ">> Sync Progress", !isSelected); - } else { - const int tocIndex = tocIndexFromItemIndex(itemIndex); - auto item = epub->getTocItem(tocIndex); + auto item = epub->getTocItem(itemIndex); - const int indentSize = 20 + (item.level - 1) * 15; - const std::string chapterName = - renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), pageWidth - 40 - indentSize); + const int indentSize = 20 + (item.level - 1) * 15; + const std::string chapterName = + renderer.truncatedText(UI_10_FONT_ID, item.title.c_str(), pageWidth - 40 - indentSize); - renderer.drawText(UI_10_FONT_ID, indentSize, displayY, chapterName.c_str(), !isSelected); - } + renderer.drawText(UI_10_FONT_ID, indentSize, displayY, chapterName.c_str(), !isSelected); } // Skip button hints in landscape CW mode (they overlap content) diff --git a/src/activities/reader/EpubReaderChapterSelectionActivity.h b/src/activities/reader/EpubReaderChapterSelectionActivity.h index 255f0cea..c43469d0 100644 --- a/src/activities/reader/EpubReaderChapterSelectionActivity.h +++ b/src/activities/reader/EpubReaderChapterSelectionActivity.h @@ -26,22 +26,12 @@ class EpubReaderChapterSelectionActivity final : public ActivityWithSubactivity // This adapts automatically when switching between portrait and landscape. int getPageItems() const; - // Total items including sync options (top and bottom) + // Total TOC items count int getTotalItems() const; - // Check if sync option is available (credentials configured) - bool hasSyncOption() const; - - // Check if given item index is a sync option (first or last) - bool isSyncItem(int index) const; - - // Convert item index to TOC index (accounting for top sync option offset) - int tocIndexFromItemIndex(int itemIndex) const; - static void taskTrampoline(void* param); [[noreturn]] void displayTaskLoop(); void renderScreen(); - void launchSyncActivity(); public: explicit EpubReaderChapterSelectionActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, diff --git a/src/activities/reader/EpubReaderMenuActivity.h b/src/activities/reader/EpubReaderMenuActivity.h index bd253f81..2145d3f0 100644 --- a/src/activities/reader/EpubReaderMenuActivity.h +++ b/src/activities/reader/EpubReaderMenuActivity.h @@ -13,7 +13,7 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { public: - enum class MenuAction { SELECT_CHAPTER, GO_HOME, DELETE_CACHE }; + enum class MenuAction { SELECT_CHAPTER, GO_HOME, SYNC, DELETE_CACHE }; explicit EpubReaderMenuActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, const std::string& title, const std::function& onBack, const std::function& onAction) @@ -34,6 +34,7 @@ class EpubReaderMenuActivity final : public ActivityWithSubactivity { const std::vector menuItems = {{MenuAction::SELECT_CHAPTER, "Go to Chapter"}, {MenuAction::GO_HOME, "Go Home"}, + {MenuAction::SYNC, "Sync Progress"}, {MenuAction::DELETE_CACHE, "Delete Book Cache"}}; int selectedIndex = 0;