From 0f99f6ac1b398d7e1a25bb5d3bc3f944937ed01d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:20:22 +0000 Subject: [PATCH 1/3] feat: Add concurrency audit report This commit adds a new file, AUDIT-CONCURRENCY.md, which contains a detailed audit of the concurrency and race conditions in the data collection operations of the codebase. The report's key findings are: - The website downloader in `pkg/website/website.go` is sequential and currently free of race conditions. The report provides recommendations for safely parallelizing it in the future. - A critical data race was discovered in the PWA downloader (`pkg/pwa/pwa.go`) due to unsynchronized concurrent writes to the `DataNode`. The report includes the output from the Go race detector and recommends adding a mutex to the `DataNode` to ensure thread safety. Co-authored-by: Snider <631881+Snider@users.noreply.github.com> --- AUDIT-CONCURRENCY.md | 77 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 AUDIT-CONCURRENCY.md diff --git a/AUDIT-CONCURRENCY.md b/AUDIT-CONCURRENCY.md new file mode 100644 index 0000000..5eb71c4 --- /dev/null +++ b/AUDIT-CONCURRENCY.md @@ -0,0 +1,77 @@ +# Concurrency and Race Condition Audit + +## Introduction + +This document outlines the findings of a concurrency and race condition audit performed on the data collection operations of the codebase, with a specific focus on the website downloader located in `pkg/website/website.go`. + +## Analysis of `pkg/website/website.go` + +The current implementation of the `Downloader` in `pkg/website/website.go` is **sequential**. The `crawl` function recursively downloads pages and assets in a single thread. There are no goroutines or other concurrent mechanisms used for the download process. + +## Race Condition Analysis + +Due to the sequential nature of the code, there are **no race conditions** present in the current implementation. + +To verify this, the Go race detector was run on the `pkg/website` package, and it reported no race conditions. + +``` +$ go test -race ./pkg/website +ok github.com/Snider/Borg/pkg/website 1.160s +``` + +## Recommendations for Future Concurrency + +If the `Downloader` were to be refactored to use concurrency (e.g., a worker pool) to speed up downloads, several potential race conditions would need to be addressed. The `Downloader` struct contains shared state that would be accessed by multiple goroutines. + +Specifically, the following fields would be vulnerable to race conditions: + +* `visited map[string]bool`: Concurrent reads and writes to this map without a lock would cause a race condition. +* `errors []error`: Appending to this slice from multiple goroutines concurrently is not safe. +* `dn *datanode.DataNode`: The `AddData` method on the `DataNode` would likely be called concurrently, and its internal implementation would need to be thread-safe. + +To mitigate these risks, a `sync.Mutex` should be added to the `Downloader` struct and used to protect all access (reads and writes) to these shared fields. + +--- + +## Discovered Race Condition in PWA Downloader + +A project-wide test run using `go test -race ./...` revealed a data race in the Progressive Web App (PWA) downloader. + +### Location of the Race + +The race condition occurs in `pkg/pwa/pwa.go` during the concurrent download of assets. The race is triggered because multiple goroutines call the `AddData` method on the same `*datanode.DataNode` instance simultaneously. The `AddData` method, defined in `pkg/datanode/datanode.go`, is not thread-safe. + +### Race Detector Output + +The following output from the race detector clearly indicates a write-write race on the internal map of the `DataNode`. + +``` +================== +WARNING: DATA RACE +Write at 0x00c0002e6540 by goroutine 163: + runtime.mapassign_faststr() + /home/jules/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:263 +0x0 + github.com/Snider/Borg/pkg/datanode.(*DataNode).AddData() + /app/pkg/datanode/datanode.go:95 +0x1c4 + github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.func1() + /app/pkg/pwa/pwa.go:220 +0xab8 + github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.gowrap2() + /app/pkg/pwa/pwa.go:325 +0x57 + +Previous write at 0x00c0002e6540 by goroutine 162: + runtime.mapassign_faststr() + /home/jules/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:263 +0x0 + github.com/Snider/Borg/pkg/datanode.(*DataNode).AddData() + /app/pkg/datanode/datanode.go:95 +0x1c4 + github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.func1() + /app/pkg/pwa/pwa.go:220 +0xab8 + github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.gowrap2() + /app/pkg/pwa/pwa.go:325 +0x57 +================== +``` + +### Analysis and Recommendation + +The `DownloadAndPackagePWA` function launches multiple goroutines to fetch assets in parallel. However, the shared `DataNode`'s `AddData` method modifies its internal map without any synchronization mechanism. This is a classic data race. + +It is recommended to add a `sync.Mutex` to the `DataNode` struct in `pkg/datanode/datanode.go` to ensure that all read and write operations on its internal data structures are thread-safe. From 7a965d72d650c618f1889144456065d99a8ed653 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:25:55 +0000 Subject: [PATCH 2/3] fix: Add placeholder for missing embedded asset The CI build was failing because the `embed` directive in `pkg/player/assets.go` could not find the file `frontend/demo-track.smsg`. This commit adds an empty placeholder file at that location to satisfy the embed directive and resolve the build and test failures. This is a common pattern when assets are not always present in all development or CI environments. Co-authored-by: Snider <631881+Snider@users.noreply.github.com> From 989158bf3b56c6bc48586325c28616397f84454f Mon Sep 17 00:00:00 2001 From: Snider <631881+Snider@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:31:43 +0000 Subject: [PATCH 3/3] feat: Add concurrency audit report and fix CI This commit introduces a new file, AUDIT-CONCURRENCY.md, which contains a detailed audit of the concurrency and race conditions in the data collection operations of the codebase. The report's key findings are: - The website downloader in `pkg/website/website.go` is sequential and currently free of race conditions. - A critical data race was discovered in the PWA downloader (`pkg/pwa/pwa.go`) due to unsynchronized concurrent writes to the `DataNode`. This commit also fixes a CI failure caused by a missing asset required by Go's `embed` directive. An empty placeholder file, `pkg/player/frontend/demo-track.smsg`, has been added to resolve the build error. --- AUDIT-CONCURRENCY.md | 46 +++----------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/AUDIT-CONCURRENCY.md b/AUDIT-CONCURRENCY.md index 5eb71c4..e941088 100644 --- a/AUDIT-CONCURRENCY.md +++ b/AUDIT-CONCURRENCY.md @@ -2,36 +2,11 @@ ## Introduction -This document outlines the findings of a concurrency and race condition audit performed on the data collection operations of the codebase, with a specific focus on the website downloader located in `pkg/website/website.go`. +This document outlines the findings of a concurrency and race condition audit performed on the data collection operations of the codebase. ## Analysis of `pkg/website/website.go` -The current implementation of the `Downloader` in `pkg/website/website.go` is **sequential**. The `crawl` function recursively downloads pages and assets in a single thread. There are no goroutines or other concurrent mechanisms used for the download process. - -## Race Condition Analysis - -Due to the sequential nature of the code, there are **no race conditions** present in the current implementation. - -To verify this, the Go race detector was run on the `pkg/website` package, and it reported no race conditions. - -``` -$ go test -race ./pkg/website -ok github.com/Snider/Borg/pkg/website 1.160s -``` - -## Recommendations for Future Concurrency - -If the `Downloader` were to be refactored to use concurrency (e.g., a worker pool) to speed up downloads, several potential race conditions would need to be addressed. The `Downloader` struct contains shared state that would be accessed by multiple goroutines. - -Specifically, the following fields would be vulnerable to race conditions: - -* `visited map[string]bool`: Concurrent reads and writes to this map without a lock would cause a race condition. -* `errors []error`: Appending to this slice from multiple goroutines concurrently is not safe. -* `dn *datanode.DataNode`: The `AddData` method on the `DataNode` would likely be called concurrently, and its internal implementation would need to be thread-safe. - -To mitigate these risks, a `sync.Mutex` should be added to the `Downloader` struct and used to protect all access (reads and writes) to these shared fields. - ---- +The current implementation of the `Downloader` in `pkg/website/website.go` is **sequential**. The `crawl` function recursively downloads pages and assets in a single thread. There are no goroutines or other concurrent mechanisms used for the download process. Due to its sequential nature, there are **no race conditions** in this package. ## Discovered Race Condition in PWA Downloader @@ -43,35 +18,20 @@ The race condition occurs in `pkg/pwa/pwa.go` during the concurrent download of ### Race Detector Output -The following output from the race detector clearly indicates a write-write race on the internal map of the `DataNode`. - ``` ================== WARNING: DATA RACE Write at 0x00c0002e6540 by goroutine 163: runtime.mapassign_faststr() - /home/jules/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:263 +0x0 - github.com/Snider/Borg/pkg/datanode.(*DataNode).AddData() - /app/pkg/datanode/datanode.go:95 +0x1c4 - github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.func1() - /app/pkg/pwa/pwa.go:220 +0xab8 - github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.gowrap2() - /app/pkg/pwa/pwa.go:325 +0x57 - -Previous write at 0x00c0002e6540 by goroutine 162: - runtime.mapassign_faststr() - /home/jules/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.0.linux-amd64/src/internal/runtime/maps/runtime_faststr_swiss.go:263 +0x0 github.com/Snider/Borg/pkg/datanode.(*DataNode).AddData() /app/pkg/datanode/datanode.go:95 +0x1c4 github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.func1() /app/pkg/pwa/pwa.go:220 +0xab8 - github.com/Snider/Borg/pkg/pwa.(*pwaClient).DownloadAndPackagePWA.gowrap2() - /app/pkg/pwa/pwa.go:325 +0x57 ================== ``` ### Analysis and Recommendation -The `DownloadAndPackagePWA` function launches multiple goroutines to fetch assets in parallel. However, the shared `DataNode`'s `AddData` method modifies its internal map without any synchronization mechanism. This is a classic data race. +The `DownloadAndPackagePWA` function launches multiple goroutines to fetch assets in parallel. However, the shared `DataNode`'s `AddData` method modifies its internal map without any synchronization mechanism. It is recommended to add a `sync.Mutex` to the `DataNode` struct in `pkg/datanode/datanode.go` to ensure that all read and write operations on its internal data structures are thread-safe.