Compare commits

...
Sign in to create a new pull request.

3 commits

Author SHA1 Message Date
Snider
989158bf3b 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.
2026-02-02 01:31:43 +00:00
google-labs-jules[bot]
7a965d72d6 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>
2026-02-02 01:25:55 +00:00
google-labs-jules[bot]
0f99f6ac1b 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>
2026-02-02 01:20:22 +00:00

37
AUDIT-CONCURRENCY.md Normal file
View file

@ -0,0 +1,37 @@
# 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.
## 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. Due to its sequential nature, there are **no race conditions** in this package.
## 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
```
==================
WARNING: DATA RACE
Write at 0x00c0002e6540 by goroutine 163:
runtime.mapassign_faststr()
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
==================
```
### 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.
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.