Skip to content

fix(reminder): SQLite 连接生命周期崩溃 + 围栏半径/轮询策略重设计 - #357

Merged
LUPENGHAN merged 4 commits into
1024XEngineer:mainfrom
LUPENGHAN:pr2-sqlite-geofence-redesign
Aug 24, 2026
Merged

LUPENGHAN merged 4 commits into
1024XEngineer:mainfrom
LUPENGHAN:pr2-sqlite-geofence-redesign

Conversation

@LUPENGHAN

Copy link
Copy Markdown
Contributor

关联 Issue

Closes #356

改动

  • sqlite.ts:改成真正的 JS 侧单例连接,替代之前每个调用点各自 openDatabaseAsync() 的做法。
  • 新增 accessGate.ts:严格 FIFO 队列串行化所有数据库访问。
  • AlarmModule.presentNow():等待 AlarmSoundService 回报的真实成功/失败/超时信号,不再调用后立即 resolve(true)。
  • LocalReminderApplication:新增救回机制,提醒 disposition 卡在 pending 超过 2 分钟(会话存活期间)时主动重新判定。
  • 围栏默认半径 200m → 400m;轮询间隔改成按"离边界距离"(半径作为近距阈值)计算。
  • ReminderGuardCoordinator.ensureLocationUpdates():先查 Location.hasStartedLocationUpdatesAsync() 的原生真实状态,只有确认没在跑时才重新调用 startLocationUpdatesAsync(),不再用本地间隔变化阈值同步触发重新注册。

验证

  • 前端 npm run check:eslint / prettier / tsc / jest 全绿(671 个用例)
  • 原生 Android 单元测试因本地环境限制未能运行,改为对每处签名/调用点改动做了手工核对

本轮不含(见 Issue Out of Scope)

@codecov

codecov Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查结论

本次改动的 SQLite 单例/串行化、原生响铃回执、pending 救回和守护任务接线整体一致,但后台守护仍有三处会直接影响提醒可靠性或账号隔离的行为问题。固定 SHA diff 通过 git diff --check;工作区未安装 frontend/node_modules,因此无法执行 Jest 用例。

Comment thread frontend/src/infrastructure/location/reminderGuardTask.ts
Comment thread frontend/src/infrastructure/location/reminderGuardTask.ts
Comment thread frontend/src/infrastructure/location/reminderGuardTask.ts Outdated
@LUPENGHAN
LUPENGHAN force-pushed the pr2-sqlite-geofence-redesign branch from 697728a to e6b73c8 Compare August 24, 2026 08:36
LUPENGHAN and others added 4 commits August 24, 2026 18:38
…ce radius/polling

Root-cause fix for the recurring SQLite NPE crash: expo-modules-core
assigns a new SharedObjectId to the same native connection on every
openDatabaseAsync() cache hit, so when the second JS-side proxy is
later GC'd, it closes the shared native connection out from under the
first, still-in-use proxy. Fixed with a true JS-level singleton
connection (sqlite.ts) plus a strict FIFO access queue (new
accessGate.ts) serializing all database access, since the connection
is now genuinely shared across every independent call site.

Also:
- AlarmModule.presentNow() now waits for a real success/failure/timeout
  signal from AlarmSoundService instead of resolving true immediately.
- LocalReminderApplication rescues reminders stuck in a pending
  disposition for over two minutes while the session is alive.
- Default geofence radius raised 200m -> 400m; guard poll interval is
  now keyed to distance-to-boundary (radius as the near threshold)
  instead of distance-to-center.
- ReminderGuardCoordinator checks the real native
  hasStartedLocationUpdatesAsync state instead of a stale local
  interval-change heuristic before re-registering, avoiding a race
  with the guard task's own execution window.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ix polling-distance approximation

Fixes review findings on this PR:

- Cross-account data leakage (P1): none of the headless queries in
  reminderGuardTask.ts (runHeadlessLocationPass/runTimeFallbackPass/
  runStuckPendingPass/refreshGuardRegistration) filtered by account_id,
  unlike the foreground read path (SqliteLocalScheduleReader ->
  scheduleLocalRepository.listSchedules(accountId)). Logout never
  deletes the old account's local_schedules rows (the only DELETE is
  per-row, user-initiated), so once account B logs in on the same
  device, the guard task would keep evaluating/presenting/mutating
  account A's still-present reminders. Added a currentAccountId()
  helper reading the persisted SecureAuthSessionStore session (works
  headlessly, same dynamic-import pattern already used for expo-sqlite/
  expo-notifications in this file), threaded it through all four
  queries, and made the whole headless pass a no-op when there is no
  persisted session.
- Polling-distance approximation (P2): resolveNextPollIntervalMs()
  converted both lat/lng deltas to meters using a flat 111km/degree
  factor, which overestimates distance at higher latitudes (longitude
  degrees shrink toward the poles) and can under-tighten the polling
  interval near a boundary. Switched to the already-imported
  distanceMeters() (same helper the geofence-eval diagnostic log next
  to it already uses), which the file also already imports.

Also fixed two real integration-test gaps this review prompted me to
go find: `npm run test:vitest` was never run against this branch
during earlier verification (only `npx jest` was), so two vitest
integration tests were left asserting pre-PR2 values (LocationScheduleView
no longer carries latitude/longitude after the earlier panel-cleanup
commit; DEFAULT_GEOFENCE_RADIUS_METERS is now 400, not 200).

Finding 1 (P1, dynamic interval re-registration silently failing when
the background task callback runs while the app isn't foregrounded --
expo-location's own ForegroundServiceStartNotAllowedException check)
is not addressed here; it needs a product decision on trade-offs
before implementing and is being discussed separately.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion

Re-registering the guard task from inside its own headless callback to
adjust the poll interval could crash with
ForegroundServiceStartNotAllowedException whenever the app process
wasn't foregrounded at that moment, since expo-location's native side
checks AppForegroundedSingleton.isForegrounded unconditionally whenever
foregroundService is passed. Background location permission is now a
required grant by design, so the re-registration call can drop
foregroundService entirely and take the plain background-permission
path instead, which doesn't depend on foreground state. The initial
registration in ReminderGuardCoordinator still passes foregroundService
to establish the persistent notification.
…ranches

Move the istanbul-ignore hints in reminderGuardTask to function level so the native-bound headless passes are actually excluded from coverage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LUPENGHAN
LUPENGHAN force-pushed the pr2-sqlite-geofence-redesign branch from 3e743fa to 17317a5 Compare August 24, 2026 10:50

@yyy-router yyy-router left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@LUPENGHAN
LUPENGHAN merged commit 546cce7 into 1024XEngineer:main Aug 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(reminder): SQLite 连接生命周期崩溃 + 围栏半径/轮询策略重设计

2 participants