fix: BackgroundWorker.compute hangs forever on a genuinely async callback - #693
Open
pandeyshivam10 wants to merge 1 commit into
Open
Conversation
…llback PdfrxComputeCallback is typed as FutureOr<R> Function(M message), but _ExecuteParams.execute() sent callback(message) directly over the SendPort. A pending Future is not a sendable isolate message, so any callback that actually awaits something (rather than happening to resolve synchronously) throws "Invalid argument(s): ... object is unsendable" inside the worker isolate, which the caller never sees -- the awaiting receivePort.first just hangs until the test/call site times out. Every existing call site happened to use a synchronous callback, so this never surfaced, but the type signature advertises support that didn't work. Fix: always await callback(message) and send back a small, definitely- sendable wrapper (_ComputeResult/_ComputeError, using the error's/stack trace's string form) instead of the raw value. _compute() unwraps it and rethrows on the caller's isolate. This also fixes a second latent gap where a *synchronous* throw inside a callback previously had no error-forwarding path either, and would have hung the same way. Added packages/pdfrx_engine/test/background_worker_test.dart covering the synchronous regression case, a genuinely-async callback with a real await, multiple sequential awaits in one callback, and both sync/async error propagation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BackgroundWorker.compute()hangs forever (never resolves, no error surfaced) when the callback passed to it does genuine asynchronous work — i.e. actually awaits something instead of completing synchronously.Root cause
_ExecuteParams.execute()ran:callbackreturnsFutureOr<R>. When it happens to complete synchronously, this works by accident, since the value is already a plainR. But when it returns a pendingFuture<R>(real async work),SendPort.send()throws:A pending
Futureis not a sendable isolate message. This throw happens inside the worker isolate, where it's swallowed — the caller'sawait receivePort.firstjust waits forever with no error ever crossing back.A related gap: a synchronous throw inside the callback also had no error-forwarding path, so that case hung too.
Fix
execute()now always awaits the callback (via anasyncclosure, so sync and async callbacks are handled the same way) and sends back one of two plain, always-sendable wrapper objects instead of the raw result:_ComputeResult<R>(value)on success_ComputeError(errorString, stackTraceString)on failure — the original error/stack trace objects aren't guaranteed sendable either (arbitrary exception types can hold non-sendable fields), so only their string forms cross the isolate boundaryThe caller side (
_compute()) checks for_ComputeErrorand rethrows a_WorkerComputeExceptionwith the reconstructed stack trace; otherwise it unwraps_ComputeResult.value.Testing
Added
test/background_worker_test.dartcovering:await)awaitsawait)All pass.
dart analyzeis clean, no regressions in the existing suite.