Skip to content

oto: close the ready channel even when Oboe initialization fails - #300

Open
kumagi wants to merge 1 commit into
ebitengine:mainfrom
kumagi:android-close-ready-on-failure
Open

kumagi wants to merge 1 commit into
ebitengine:mainfrom
kumagi:android-close-ready-on-failure

Conversation

@kumagi

@kumagi kumagi commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

What issue is this addressing?

No issue filed yet. Found during an audit of the Android driver.

What type of issue is this addressing?

bug

What this PR does | solves

In newContext (driver_android.go), the ready channel is closed only after oboe.Play succeeds:

go func() {
    c.m.Lock()
    defer c.m.Unlock()

    if err := oboe.Play(sampleRate, channelCount, c.mux.ReadFloat32s, c.err.Join, bufferSizeInBytes); err != nil {
        c.err.Join(err)
        return              // <- returns without closing ready
    }
    close(ready)
}()

Callers of NewContext wait on <-ready. When oboe.Play fails — e.g. when neither AAudio nor OpenSL ES can open a stream — the goroutine returns with the channel still open, so the caller blocks forever instead of observing the stored error.

The fix

Close ready unconditionally via defer:

defer close(ready)

c.m.Lock()
defer c.m.Unlock()

if err := oboe.Play(...); err != nil {
    c.err.Join(err)
}

The error is still recorded with c.err.Join and reported through Context.Err; only the missing wakeup is fixed.

Otherwise NewContext callers waiting on the channel block forever when
oboe.Play reports an error.
@hajimehoshi

Copy link
Copy Markdown
Member

Reviewed the change and checked the other drivers for similar channel-completion issues. No correctness issues found in this PR: the initialization error is recorded and the mutex is released before ready closes.

Windows and Unix already defer closing their readiness channels, Darwin explicitly closes on both initialization success and failure, and the console driver closes immediately.

There is a separate follow-up in driver_js.go: both AudioContext.resume() and audioWorklet.addModule() have success handlers but no rejection handlers. A rejected initial resume() leaves ready open, while a failed addModule() can still be followed by a successful readiness signal because readiness currently depends only on resume(). Context.Err() also always returns nil there. A follow-up should record initialization failures and complete readiness exactly once, coordinating worklet setup with resume while preserving the intentional wait for user interaction.

Static review only; Android and browser failure scenarios were not run.

Posted by Codex (OpenAI), on behalf of @hajimehoshi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants