Skip to content

fix: problems with rendering and detached threads - #1229

Open
mdydek wants to merge 6 commits into
mainfrom
fix/multi-threaded-problems
Open

fix: problems with rendering and detached threads#1229
mdydek wants to merge 6 commits into
mainfrom
fix/multi-threaded-problems

Conversation

@mdydek

@mdydek mdydek commented Aug 14, 2026

Copy link
Copy Markdown
Member

Closes #

⚠️ Breaking changes ⚠️

Introduced changes

  • fast suspend and resume in offline audio context could have a slight window when two rendering threads were running in the same time, which could result in multiple consumers of the events
  • removing node lacked self draining
  • promise offloader was destroyed too late in the context destructor which could result in use after free
  • do not create new threads on audio thread, which results in allocation, changed logic or place of creation new thread

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added/Conducted relevant tests
  • Performed self-review of the code
  • Updated Web Audio API coverage
  • Added support for web
  • Updated old arch android spec file

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

WPT non-regression comparison

ERROR — the comparison did not produce a report; the test run itself likely failed.

Workflow run · this comment is updated on every push.

@mdydek mdydek added the fix Code changes specifically addressing and resolving a bug label Aug 14, 2026
@mdydek
mdydek marked this pull request as ready for review August 18, 2026 13:47
Comment on lines +121 to +129
seekDecoderDaemon_ = std::make_unique<SeekDecoderDaemon>(
auto deamon = std::make_unique<SeekDecoderDaemon>(
std::move(daemonOptions),
decoderState_,
std::move(commandReceiver),
std::move(frameSender),
frameReceiver_);

seekDecoderThread_ = std::thread(std::move(*deamon));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SeekDecoderDaemon should own the thread, not the AudioFileSourceNode since that is an internal property of the daemon worker.

Comment on lines 185 to 203

struct DeferredEmptyEvent {
double dueTime;
AudioEvent event;
uint64_t callbackId;
};

/// Reserved up front so `deferEmptyEventDispatch` stays allocation-free on
/// the audio thread until this many dispatches are pending at once.
static constexpr size_t DEFERRED_EMPTY_EVENTS_CAPACITY = 16;

/// Render-serialized only (audio thread or the synchronous
/// `scheduleAudioEvent` path) — no lock, so no other thread may touch it.
std::vector<DeferredEmptyEvent> deferredEmptyEvents_;

void dispatchDueDeferredEvents();

[[nodiscard]] virtual bool isDriverRunning() const = 0;
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does all this wiring belong to AudioContext? Besides can't we reuse some implementation like BoundedPriorityQueue?

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

Labels

fix Code changes specifically addressing and resolving a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants