fix: races

[gjermundgaraba]

Fixes two races found by running go test -race

1st: luaplugin: timer callbacks can enter the same Lua VM concurrently with plugin loading, so gopher-lua sees unsynchronized access to one *lua.LState. Fixed by plugin loading now holding the same mutex that timer callbacks use, so the Lua VM can’t be entered concurrently during startup. Cleanup also stops a plugin’s timers so callbacks can’t fire after failed or skipped loads.

Test output:

cliamp/luaplugin.registerTimerAPI.func1.1()
    /Users/gg/code/wannabe-worktrees/cliamp/herpderp/luaplugin/api_timer.go:87 +0x12c

Previous write at ... by goroutine 55:
github.com/yuin/gopher-lua.(*LState).DoFile()
    /Users/gg/go/pkg/mod/github.com/yuin/gopher-lua@v1.1.2/auxlib.go:401 +0xa0
cliamp/luaplugin.(*Manager).loadPlugin()
    /Users/gg/code/wannabe-worktrees/cliamp/herpderp/luaplugin/luaplugin.go:197 +0x194

--- FAIL: TestTimerAfter (0.15s)
    testing.go:1712: race detected during execution of test

2nd: ui/model: there is a race in the oto v3.3.x (coming from beep). oto v3.4.0 does not have this issue. So, fix was just a bump. I have also opened a PR to beep to bump oto gopxl/beep#222

Test output:

Write at ... by goroutine 10:
github.com/ebitengine/oto/v3.newContext.func1()
    /Users/gg/go/pkg/mod/github.com/ebitengine/oto/v3@v3.3.2/driver_darwin.go:161 +0x2e0

Previous read at ... by main goroutine:
github.com/ebitengine/oto/v3.newContext()
    /Users/gg/go/pkg/mod/github.com/ebitengine/oto/v3@v3.3.2/driver_darwin.go:166 +0x318
cliamp/ui/model.TestMain()
    /Users/gg/code/wannabe-worktrees/cliamp/herpderp/ui/model/tick_test.go:22 +0x50

testing: race detected outside of test execution

Summary by CodeRabbit

• Chores

  • Updated Go module dependencies for improved stability.

• Bug Fixes

  • Improved plugin cleanup to reliably remove timers, hooks, and visualizers when plugins fail to load or don't register.
  • Enhanced timer management to prevent callbacks from running after a plugin is removed.

• Tests

  • Added tests verifying plugin cleanup and that timers/hooks/visualizers are removed on load errors.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d60a7c66-82e7-4a39-9508-8cd0568a7f5f

📥 Commits

Reviewing files that changed from the base of the PR and between 8b0344c and 070ed9a.

📒 Files selected for processing (2)

• luaplugin/luaplugin.go
• luaplugin/luaplugin_test.go

---

📝 Walkthrough

Walkthrough

Centralized plugin cleanup on load failure/non-registration, added plugin-scoped timer controls and safer timer goroutine flow, bumped two indirect Go module versions, and added tests verifying timers, hooks, and visualizers are removed when plugin load fails.

Changes

Cohort / File(s)	Summary
Go Dependencies go.mod	Bumped indirect dependencies: github.com/ebitengine/oto/v3 v3.3.2 → v3.4.0, github.com/ebitengine/purego v0.8.0 → v0.9.0.
Timer Management luaplugin/api_timer.go	Added stopPlugin(p *Plugin), take(id int64) bool, and active(id int64) bool. Changed timer goroutine logic to check/remove entries atomically and exit if the entry was removed, preventing callbacks after cleanup.
Plugin Loading / Cleanup luaplugin/luaplugin.go	Added cleanupPlugin(p *Plugin) to remove plugin entries from m.hooks, m.visPlugs, m.visMap and stop plugin timers. Reworked Manager.loadPlugin locking to call cleanup on DoFile errors or when plugin doesn't register.
Tests luaplugin/luaplugin_test.go	Added helper loadTestPluginExpectError and tests: TestLoadPluginCleanupStopsPendingTimers, TestLoadPluginErrorRemovesHooks, TestLoadVisualizerErrorRemovesVisualizer to assert timers/hooks/visualizers are removed on load failure or non-registration.

Sequence Diagram(s)

sequenceDiagram
  participant Manager
  participant Plugin
  participant LuaState
  participant TimerGoroutine

  Manager->>Plugin: create Plugin instance
  Manager->>LuaState: L.DoFile(path) (holds p.mu)
  LuaState-->>Plugin: executes script (may register timers/hooks/visualizer)
  LuaState-->>Manager: register calls (hooks, visPlugs, visMap, timers)
  alt DoFile succeeds and plugin registers
    Manager->>Plugin: release p.mu (normal lifecycle)
  else DoFile fails or plugin doesn't register
    Manager->>Manager: cleanupPlugin(p)
    Manager->>TimerGoroutine: stopPlugin(p) (remove timer entries)
    TimerGoroutine-->>TimerGoroutine: timers check tm.take/tm.active and exit without invoking callbacks
    Manager->>Manager: remove hooks, visPlugs, visMap entries
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix: races' is vague and generic, using non-descriptive language that does not convey meaningful information about the specific changes in this complex pull request.	Replace with a more specific title like 'fix: data races in timer and audio initialization' or 'fix: concurrent Lua VM access and oto dependency race' to clearly indicate the primary changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@luaplugin/luaplugin_test.go`:
- Around line 108-159: Add a new test case in
TestLoadPluginCleanupStopsPendingTimers that exercises recurring timers via
cliamp.timer.every so the tm.active(id) bail-out path is covered: extend the
test's cases slice with a case (e.g., name "every") whose code uses
cliamp.timer.every(0.01, function() cliamp.fs.write(%q, "fired") end) followed
by cliamp.sleep(0.05) (mirroring the existing after-case), and run it in the
same t.Run flow (expecting no plugin for the no-register path or using expectErr
where appropriate) so the test asserts the recurring-timer callback is not
invoked after cleanup. Ensure the new case uses the same path formatting and
post-check (os.Stat) as the other cases.

In `@luaplugin/luaplugin.go`:
- Around line 220-236: The slices m.hooks and m.visPlugs currently reuse backing
arrays via hooks[:0] and m.visPlugs[:0], leaving removed *luaHook and *luaVis
pointers in the unused tail and preventing GC; update the removal code to zero
out the dropped tail elements before reassigning (for each original slice
iterate the old range after filtering and set remaining slots to nil or
explicitly clear entries in the old slice) so that removed luaHook/luaVis values
and the failed plugin p are not retained; reference m.hooks, hooks, *luaHook,
m.visPlugs, vis and *luaVis when making the change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: afba1601-be5a-4938-b82a-162832cfb932

📥 Commits

Reviewing files that changed from the base of the PR and between cdfe752 and 8b0344c.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (4)

• go.mod
• luaplugin/api_timer.go
• luaplugin/luaplugin.go
• luaplugin/luaplugin_test.go