mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-26 04:19:27 -07:00
fix(mpv): avoid crash notification on video close
This commit is contained in:
@@ -0,0 +1,54 @@
|
|||||||
|
---
|
||||||
|
id: TASK-296
|
||||||
|
title: Suppress crash notification when closing launcher-managed video
|
||||||
|
status: Done
|
||||||
|
assignee: []
|
||||||
|
created_date: '2026-04-25 23:12'
|
||||||
|
updated_date: '2026-04-26 02:44'
|
||||||
|
labels:
|
||||||
|
- bug
|
||||||
|
- launcher
|
||||||
|
- mpv
|
||||||
|
dependencies: []
|
||||||
|
priority: high
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
Investigate and fix regression where closing a running mpv video causes SubMiner/Electron service crash notification (`<html><tt>/SubMiner</tt> has encountered a fatal error and was closed.</html>`). Not present on origin/main/v0.12.0 path.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [x] #1 Closing a launcher-managed video stops the overlay/app without desktop crash notification.
|
||||||
|
- [x] #2 Regression test covers the shutdown path that caused the notification.
|
||||||
|
- [x] #3 Relevant launcher/app tests pass.
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Implementation Plan
|
||||||
|
|
||||||
|
<!-- SECTION:PLAN:BEGIN -->
|
||||||
|
1. Confirm notification source from local crash metadata and mpv/app logs.
|
||||||
|
2. Add regression coverage for mpv quit/shutdown lifecycle helper spawning.
|
||||||
|
3. Update mpv Lua lifecycle to avoid Electron helper subprocesses during quit/shutdown while preserving normal end-file hide behavior.
|
||||||
|
4. Run plugin tests and changelog lint.
|
||||||
|
<!-- SECTION:PLAN:END -->
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
<!-- SECTION:NOTES:BEGIN -->
|
||||||
|
Crash records in ~/.cache/drkonqi show SIGBUS in Electron NetworkService utility process for SubMiner AppImage. mpv log shows shutdown starts two `/home/sudacode/.local/bin/SubMiner.AppImage --hide-visible-overlay` subprocesses and kills them during close. Root cause is mpv plugin spawning Electron control helpers during quit/shutdown.
|
||||||
|
|
||||||
|
Follow-up after retest: installed plugin matched source patch and no close-time hide command was spawned. New mpv log shows the initial `/home/sudacode/.local/bin/SubMiner.AppImage --start ...` subprocess remains owned by mpv for the whole playback and is killed when mpv quits. New DrKonqi crash at 2026-04-25 16:44 again shows SIGBUS in Electron NetworkService from that AppImage mount. Need detach the long-lived plugin-launched `--start` app process from mpv.
|
||||||
|
|
||||||
|
Second fix: plugin-launched `--start` now includes `--background`, using SubMiner's existing background relaunch path so mpv owns only a short-lived starter process rather than the long-running Electron app. Ran `make install-plugin` so ~/.config/mpv/scripts/subminer now contains both lifecycle and background-start fixes. Full `scripts/test-plugin-start-gate.lua` is currently blocked by an unrelated dirty-worktree primary subtitle bar binding test from TASK-297.
|
||||||
|
<!-- SECTION:NOTES:END -->
|
||||||
|
|
||||||
|
## Final Summary
|
||||||
|
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||||
|
Changed the mpv Lua lifecycle so `shutdown` no longer spawns a `--hide-visible-overlay` helper, and `end-file` skips that helper when mpv reports `reason = "quit"`. Also changed plugin-launched `--start` to include `--background`, so mpv owns only SubMiner's short background launcher process instead of the long-running Electron/AppImage process. This addresses both observed crash sources: close-time helper commands and mpv killing the main SubMiner child process at quit. Installed the updated plugin into `~/.config/mpv/scripts/subminer` with `make install-plugin`, and the user confirmed the latest close no longer produced the notification.
|
||||||
|
|
||||||
|
Tests: `lua scripts/test-plugin-start-gate.lua` initially proved the shutdown regression failed before the lifecycle fix; full start-gate is currently affected by other dirty work in this file. Passing checks for this commit: `lua scripts/test-plugin-lua-compat.lua && lua scripts/test-plugin-binary-windows.lua`; `bun run changelog:lint`.
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||||
4
changes/296-mpv-close-crash-notification.md
Normal file
4
changes/296-mpv-close-crash-notification.md
Normal file
@@ -0,0 +1,4 @@
|
|||||||
|
type: fixed
|
||||||
|
area: mpv
|
||||||
|
|
||||||
|
- Stopped mpv from owning long-running SubMiner AppImage subprocesses during playback shutdown, preventing desktop crash notifications when closing video.
|
||||||
@@ -73,10 +73,6 @@ function M.create(ctx)
|
|||||||
aniskip.clear_aniskip_state()
|
aniskip.clear_aniskip_state()
|
||||||
hover.clear_hover_overlay()
|
hover.clear_hover_overlay()
|
||||||
process.disarm_auto_play_ready_gate()
|
process.disarm_auto_play_ready_gate()
|
||||||
if state.overlay_running then
|
|
||||||
subminer_log("info", "lifecycle", "mpv shutting down, hiding SubMiner overlay")
|
|
||||||
process.hide_visible_overlay()
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
local function register_lifecycle_hooks()
|
local function register_lifecycle_hooks()
|
||||||
@@ -85,10 +81,11 @@ function M.create(ctx)
|
|||||||
mp.register_event("file-loaded", function()
|
mp.register_event("file-loaded", function()
|
||||||
hover.clear_hover_overlay()
|
hover.clear_hover_overlay()
|
||||||
end)
|
end)
|
||||||
mp.register_event("end-file", function()
|
mp.register_event("end-file", function(event)
|
||||||
process.disarm_auto_play_ready_gate()
|
process.disarm_auto_play_ready_gate()
|
||||||
hover.clear_hover_overlay()
|
hover.clear_hover_overlay()
|
||||||
if state.overlay_running then
|
local reason = type(event) == "table" and event.reason or nil
|
||||||
|
if state.overlay_running and reason ~= "quit" then
|
||||||
process.hide_visible_overlay()
|
process.hide_visible_overlay()
|
||||||
end
|
end
|
||||||
end)
|
end)
|
||||||
|
|||||||
@@ -186,6 +186,8 @@ function M.create(ctx)
|
|||||||
end
|
end
|
||||||
|
|
||||||
if action == "start" then
|
if action == "start" then
|
||||||
|
table.insert(args, "--background")
|
||||||
|
|
||||||
local backend = resolve_backend(overrides.backend)
|
local backend = resolve_backend(overrides.backend)
|
||||||
if backend and backend ~= "" then
|
if backend and backend ~= "" then
|
||||||
table.insert(args, "--backend")
|
table.insert(args, "--backend")
|
||||||
|
|||||||
@@ -499,10 +499,10 @@ local function count_property_set(property_sets, name, value)
|
|||||||
return count
|
return count
|
||||||
end
|
end
|
||||||
|
|
||||||
local function fire_event(recorded, name)
|
local function fire_event(recorded, name, ...)
|
||||||
local listeners = recorded.events[name] or {}
|
local listeners = recorded.events[name] or {}
|
||||||
for _, listener in ipairs(listeners) do
|
for _, listener in ipairs(listeners) do
|
||||||
listener()
|
listener(...)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -768,6 +768,7 @@ do
|
|||||||
fire_event(recorded, "file-loaded")
|
fire_event(recorded, "file-loaded")
|
||||||
local start_call = find_start_call(recorded.async_calls)
|
local start_call = find_start_call(recorded.async_calls)
|
||||||
assert_true(start_call ~= nil, "auto-start should issue --start command")
|
assert_true(start_call ~= nil, "auto-start should issue --start command")
|
||||||
|
assert_true(call_has_arg(start_call, "--background"), "auto-start should launch SubMiner in background mode")
|
||||||
assert_true(call_has_arg(start_call, "--texthooker"), "auto-start should include --texthooker on the main --start command when enabled")
|
assert_true(call_has_arg(start_call, "--texthooker"), "auto-start should include --texthooker on the main --start command when enabled")
|
||||||
assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "auto-start should not issue a separate texthooker helper command")
|
assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "auto-start should not issue a separate texthooker helper command")
|
||||||
assert_true(
|
assert_true(
|
||||||
@@ -1054,11 +1055,20 @@ do
|
|||||||
})
|
})
|
||||||
assert_true(recorded ~= nil, "plugin failed to load for shutdown-preserve-background scenario: " .. tostring(err))
|
assert_true(recorded ~= nil, "plugin failed to load for shutdown-preserve-background scenario: " .. tostring(err))
|
||||||
fire_event(recorded, "file-loaded")
|
fire_event(recorded, "file-loaded")
|
||||||
|
fire_event(recorded, "end-file", { reason = "quit" })
|
||||||
|
assert_true(
|
||||||
|
find_control_call(recorded.async_calls, "--hide-visible-overlay") == nil,
|
||||||
|
"mpv quit end-file should not spawn hide-visible-overlay helper commands"
|
||||||
|
)
|
||||||
fire_event(recorded, "shutdown")
|
fire_event(recorded, "shutdown")
|
||||||
assert_true(
|
assert_true(
|
||||||
find_control_call(recorded.async_calls, "--stop") == nil,
|
find_control_call(recorded.async_calls, "--stop") == nil,
|
||||||
"mpv shutdown should not stop the background SubMiner process"
|
"mpv shutdown should not stop the background SubMiner process"
|
||||||
)
|
)
|
||||||
|
assert_true(
|
||||||
|
find_control_call(recorded.async_calls, "--hide-visible-overlay") == nil,
|
||||||
|
"mpv shutdown should not spawn hide-visible-overlay helper commands"
|
||||||
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
do
|
do
|
||||||
|
|||||||
Reference in New Issue
Block a user