Skip to content

[Bug]: race condition in runBatch allows duplicate concurrent batches on rapid clicks #14

Description

@brianckeegan

What happened?

Type: bug
Affected files: src/modules/menu.ts
Severity: medium (data-corruption-class on the progress window; not on the Zotero DB, which is protected by withDbLock)

Summary

runBatch in src/modules/menu.ts guards against concurrent batches with addon.data.batchInFlight, but the guard check and the assignment that sets the flag are separated by an await preflightServer(). Two rapid clicks of "Convert with Docling" both pass the check, both await preflight, and both proceed to start a batch — orphaning the first batch's managed progress window and double-counting completion toasts.

Root cause

// src/modules/menu.ts, runBatch()
if (addon.data.batchInFlight) {              // <-- check
  toast("Docling", "A conversion batch is already running …", false);
  return;
}

if (!(await preflightServer())) {            // <-- await opens the race window
  toast("Docling", "docling-serve isn't running — start it and retry", false);
  return;
}

addon.data.batchInFlight = true;             // <-- set, too late

The notifier in src/modules/notifier.ts already implements the correct pattern: set the flag immediately after the check, before any await. See processPending's use of the processing flag.

Proposed fix

Set addon.data.batchInFlight = true immediately after the if (addon.data.batchInFlight) check and before the await preflightServer() call. The existing try { … } finally { addon.data.batchInFlight = false; } block at the end of runBatch handles the cleanup correctly; the only adjustment needed is to also clear the flag in the preflight-failed early return.

if (addon.data.batchInFlight) {
  toast("Docling", "A conversion batch is already running …", false);
  return;
}
addon.data.batchInFlight = true;

try {
  if (!(await preflightServer())) {
    toast("Docling", "docling-serve isn't running — start it and retry", false);
    return;
  }
  // … rest of the function body, minus the existing `addon.data.batchInFlight = true;`
} finally {
  addon.data.batchInFlight = false;
}

Acceptance criteria

  • Two rapid clicks of "Convert with Docling" within ~200 ms result in exactly one batch starting and one toast on the second click.
  • The existing pre-flight, "no PDFs in selection", and force-filter early returns all still correctly release batchInFlight (or never set it, depending on chosen structuring).
  • No regression in the notifier.ts parallel path, which currently relies on batchInFlight being set by runBatch for its deferral logic.

Related

  • The inFlightItems per-item set in convert.ts provides a second line of defence at the item level, but it does not prevent the duplicate progress-window orchestration — only the menu-level flag does.

What did you expect to happen?

See above

Steps to reproduce

  1. Select a parent item containing at least one PDF that has not yet been converted.
  2. Open the right-click menu and click "Convert with Docling".
  3. Immediately open the menu again and click "Convert with Docling" a second time before the first preflight completes (~50–200 ms window depending on local network).
  4. Observe in Browser Toolbox console (filtered for [Docling/menu]) that two batches initialize, and that startManagedProgress is called twice in a row — the second call closes the first progress window and starts fresh.

Plugin version

0.2.1

Zotero version

9.03

docling-serve version

No response

Operating system

macOS

Transport mode

None

Pipeline

None

Relevant logs


Anything else?

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions