fix(webkit): isolate download state per-download to prevent race conditions
- commit
- f291bc4e8c07cbf034ae20e2d15259feba7944e6
- author
- bnema · · 2026-01-08 17:40 UTC
- parent
- dd58ad7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +16 -9
M internal/infrastructure/webkit/download_handler.go +16 -9
| @@ -33,6 +33,13 @@ func NewDownloadHandler(downloadPath string, handler port.DownloadEventHandler) | |||
| 33 | 33 | } | |
| 34 | 34 | } | |
| 35 | 35 | ||
| 36 | + | // downloadState tracks the state of a single download to prevent duplicate notifications. | |
| 37 | + | // This is allocated per-download to ensure proper isolation between concurrent downloads. | |
| 38 | + | type downloadState struct { | |
| 39 | + | failed bool | |
| 40 | + | mu sync.Mutex | |
| 41 | + | } | |
| 42 | + | ||
| 36 | 43 | // HandleDownload sets up signal handlers for a new download. | |
| 37 | 44 | func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.Download) { | |
| 38 | 45 | log := logging.FromContext(ctx) | |
| @@ -42,9 +49,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||
| 42 | 49 | eventHandler := h.eventHandler | |
| 43 | 50 | h.mu.RUnlock() | |
| 44 | 51 | ||
| 45 | - | // Track if download failed (to avoid duplicate finished notifications). | |
| 46 | - | var downloadFailed bool | |
| 47 | - | var failedMu sync.Mutex | |
| 52 | + | // Track download state in a dedicated struct to ensure proper isolation. | |
| 53 | + | // Each download gets its own state to prevent interference between concurrent downloads. | |
| 54 | + | state := &downloadState{} | |
| 48 | 55 | ||
| 49 | 56 | // Handle decide-destination signal to set download path. | |
| 50 | 57 | //nolint:unparam // bool return is required by WebKit's signal signature (false = handled synchronously) | |
| @@ -89,9 +96,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||
| 89 | 96 | ||
| 90 | 97 | // Handle failed signal. | |
| 91 | 98 | failedCb := func(d webkit.Download, _ uintptr) { | |
| 92 | - | failedMu.Lock() | |
| 93 | - | downloadFailed = true | |
| 94 | - | failedMu.Unlock() | |
| 99 | + | state.mu.Lock() | |
| 100 | + | state.failed = true | |
| 101 | + | state.mu.Unlock() | |
| 95 | 102 | ||
| 96 | 103 | dest := d.GetDestination() | |
| 97 | 104 | filename := extractFilename(dest) | |
| @@ -114,9 +121,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||
| 114 | 121 | ||
| 115 | 122 | // Handle finished signal. | |
| 116 | 123 | finishedCb := func(d webkit.Download) { | |
| 117 | - | failedMu.Lock() | |
| 118 | - | failed := downloadFailed | |
| 119 | - | failedMu.Unlock() | |
| 124 | + | state.mu.Lock() | |
| 125 | + | failed := state.failed | |
| 126 | + | state.mu.Unlock() | |
| 120 | 127 | ||
| 121 | 128 | // Don't send finished event if we already sent a failed event. | |
| 122 | 129 | if failed { | |
| @@ -33,6 +33,13 @@ func NewDownloadHandler(downloadPath string, handler port.DownloadEventHandler) | |||||
| 33 | } | 33 | } | ||
| 34 | } | 34 | } | ||
| 35 | 35 | ||||
| 36 | + | // downloadState tracks the state of a single download to prevent duplicate notifications. | |||
| 37 | + | // This is allocated per-download to ensure proper isolation between concurrent downloads. | |||
| 38 | + | type downloadState struct { | |||
| 39 | + | failed bool | |||
| 40 | + | mu sync.Mutex | |||
| 41 | + | } | |||
| 42 | + | ||||
| 36 | // HandleDownload sets up signal handlers for a new download. | 43 | // HandleDownload sets up signal handlers for a new download. | ||
| 37 | func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.Download) { | 44 | func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.Download) { | ||
| 38 | log := logging.FromContext(ctx) | 45 | log := logging.FromContext(ctx) | ||
| @@ -42,9 +49,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||||
| 42 | eventHandler := h.eventHandler | 49 | eventHandler := h.eventHandler | ||
| 43 | h.mu.RUnlock() | 50 | h.mu.RUnlock() | ||
| 44 | 51 | ||||
| 45 | - | // Track if download failed (to avoid duplicate finished notifications). | 52 | + | // Track download state in a dedicated struct to ensure proper isolation. |
| 46 | - | var downloadFailed bool | 53 | + | // Each download gets its own state to prevent interference between concurrent downloads. |
| 47 | - | var failedMu sync.Mutex | 54 | + | state := &downloadState{} |
| 48 | 55 | ||||
| 49 | // Handle decide-destination signal to set download path. | 56 | // Handle decide-destination signal to set download path. | ||
| 50 | //nolint:unparam // bool return is required by WebKit's signal signature (false = handled synchronously) | 57 | //nolint:unparam // bool return is required by WebKit's signal signature (false = handled synchronously) | ||
| @@ -89,9 +96,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||||
| 89 | 96 | ||||
| 90 | // Handle failed signal. | 97 | // Handle failed signal. | ||
| 91 | failedCb := func(d webkit.Download, _ uintptr) { | 98 | failedCb := func(d webkit.Download, _ uintptr) { | ||
| 92 | - | failedMu.Lock() | 99 | + | state.mu.Lock() |
| 93 | - | downloadFailed = true | 100 | + | state.failed = true |
| 94 | - | failedMu.Unlock() | 101 | + | state.mu.Unlock() |
| 95 | 102 | ||||
| 96 | dest := d.GetDestination() | 103 | dest := d.GetDestination() | ||
| 97 | filename := extractFilename(dest) | 104 | filename := extractFilename(dest) | ||
| @@ -114,9 +121,9 @@ func (h *DownloadHandler) HandleDownload(ctx context.Context, download *webkit.D | |||||
| 114 | 121 | ||||
| 115 | // Handle finished signal. | 122 | // Handle finished signal. | ||
| 116 | finishedCb := func(d webkit.Download) { | 123 | finishedCb := func(d webkit.Download) { | ||
| 117 | - | failedMu.Lock() | 124 | + | state.mu.Lock() |
| 118 | - | failed := downloadFailed | 125 | + | failed := state.failed |
| 119 | - | failedMu.Unlock() | 126 | + | state.mu.Unlock() |
| 120 | 127 | ||||
| 121 | // Don't send finished event if we already sent a failed event. | 128 | // Don't send finished event if we already sent a failed event. | ||
| 122 | if failed { | 129 | if failed { | ||