| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Shutdown does not actively wait for HEALTH to leave sqlite3_step: service_wait_exit(SERVICE_HEALTH) is bounded to 3s, cancel_main_threads only joins threads already marked EXITED, and nd_thread_join_threads reaps the exited list rather than blocking on live threads. If HEALTH is still iterating SQL_LOAD_HEALTH_LOG when sqlite_close_databases() runs, sqlite3_close_v2() tears down the page cache underneath it and the next pcache1Unpin faults at offset 0x30. Bail the row loop on !service_running(SERVICE_HEALTH), matching the existing pattern at sqlite_health.c:567. Also move rw_spinlock_init() before sql_health_alarm_log_load() so the lock the load already takes is initialized first.
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
sequenceDiagram
participant HEALTH as HEALTH Service
participant INIT as health_initialize_rrdhost()
participant LOAD as sql_health_alarm_log_load()
participant DB as SQLite Database
participant SPIN as health_log.spinlock
Note over HEALTH,DB: HEALTH startup flow (current)
INIT->>SPIN: rw_spinlock_init(&spinlock)
Note over INIT,SPIN: (CHANGED order: now before load)
INIT->>LOAD: sql_health_alarm_log_load(host)
LOAD->>SPIN: rw_spinlock_read_lock()
LOAD->>DB: sqlite3_step_monitored(res)
DB-->>LOAD: SQLITE_ROW (row data)
loop For each row
LOAD->>HEALTH: service_running(SERVICE_HEALTH)?
alt HEALTH is still running
LOAD->>LOAD: Process row, create ALARM_ENTRY
LOAD->>DB: sqlite3_step_monitored(res) for next row
DB-->>LOAD: SQLITE_ROW or SQLITE_DONE
else HEALTH signaled to stop
LOAD->>LOAD: Exit loop early
Note over LOAD: (prevents access after DB close)
end
end
LOAD->>SPIN: rw_spinlock_read_unlock()
LOAD-->>INIT: Return
Note over HEALTH,DB: HEALTH shutdown sequence (race fixed)
HEALTH->>HEALTH: service_signal_exit(SERVICE_HEALTH)
HEALTH->>DB: sqlite_close_databases()
Note over HEALTH,DB: LOAD loop already exited, no dangling step()
Sorry, something went wrong.
There was a problem hiding this comment.
Fixes a shutdown-time race in the HEALTH service where SQLite could be closed while alert log restoration is still iterating rows, and ensures the health log spinlock is initialized before it’s used during load.
Changes:
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/health/health_event_loop.c | Moves health log spinlock initialization earlier so the DB restore path can safely take the lock. |
| src/database/sqlite/sqlite_health.c | Adds service_running(SERVICE_HEALTH) to the row-iteration loop condition to exit early during shutdown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
Sorry, something went wrong.
There was a problem hiding this comment.
No issues happen during shutdown and runtime. LGTM!
Sorry, something went wrong.
…22448) * fix(health): close shutdown race in sql_health_alarm_log_load Shutdown does not actively wait for HEALTH to leave sqlite3_step: service_wait_exit(SERVICE_HEALTH) is bounded to 3s, cancel_main_threads only joins threads already marked EXITED, and nd_thread_join_threads reaps the exited list rather than blocking on live threads. If HEALTH is still iterating SQL_LOAD_HEALTH_LOG when sqlite_close_databases() runs, sqlite3_close_v2() tears down the page cache underneath it and the next pcache1Unpin faults at offset 0x30. Bail the row loop on !service_running(SERVICE_HEALTH), matching the existing pattern at sqlite_health.c:567. Also move rw_spinlock_init() before sql_health_alarm_log_load() so the lock the load already takes is initialized first. * fix(health): change spinlock to write lock for health log access (cherry picked from commit 5c96c3e)
| Back | FazBrowse Home | New Git URL |
Summary
Summary by cubic
Fixes a shutdown race in the HEALTH service that could crash while restoring alert logs if SQLite closes mid-iteration. Also switches health log access to a write lock during load to avoid concurrent writes.
Written for commit 371b50e. Summary will update on new commits.