An operation ends when the client says so, not when its status says done

fprintd's contract: a status with done=true means no more status is coming,
not that the operation is over. The client still calls EnrollStop or
VerifyStop, and until it does the device is busy with that operation. Clearing
the op on done made every stock client's Stop fail with NoActionInProgress --
seen on the first fprintd-enroll against this daemon, which otherwise succeeded
through all ten stages.

Clearing it from the worker was also a race: a client that cancels and
immediately starts a new operation would have had that new operation cleared by
the old one's completion event. The worker no longer touches the op state at
all; only Stop and Release do, on the main thread.
This commit is contained in:
Jorijn van der Graaf 2026-09-02 22:19:08 +02:00
commit 0809b95a48

View file

@ -1330,7 +1330,7 @@ struct Job {
// Everything the worker sends back to the main thread. Delivered by g_idle_add // Everything the worker sends back to the main thread. Delivered by g_idle_add
// so the D-Bus emission happens on the thread that owns the connection. // so the D-Bus emission happens on the thread that owns the connection.
struct Event { struct Event {
enum class Kind { Ready, StartFailed, ClaimDone, EnrollStatus, VerifyStatus, OpFinished } kind; enum class Kind { Ready, StartFailed, ClaimDone, EnrollStatus, VerifyStatus } kind;
bool ok = false; bool ok = false;
bool done = false; bool done = false;
std::string status; std::string status;
@ -1430,7 +1430,6 @@ private:
else ev->status = "enroll-failed"; else ev->status = "enroll-failed";
if (!o.why.empty()) std::println("enrolment: {}", o.why); if (!o.why.empty()) std::println("enrolment: {}", o.why);
PostEvent(std::move(ev)); PostEvent(std::move(ev));
PostEvent(std::make_unique<Event>(Event{ .kind = Event::Kind::OpFinished }));
break; break;
} }
case Job::Kind::Verify: { case Job::Kind::Verify: {
@ -1443,7 +1442,6 @@ private:
else if (o.matched) ev->status = "verify-match"; else if (o.matched) ev->status = "verify-match";
else ev->status = "verify-no-match"; else ev->status = "verify-no-match";
PostEvent(std::move(ev)); PostEvent(std::move(ev));
PostEvent(std::make_unique<Event>(Event{ .kind = Event::Kind::OpFinished }));
break; break;
} }
} }
@ -1646,10 +1644,6 @@ void PostEvent(std::unique_ptr<Event> ev) {
if (ev->done && ev->status == "verify-match") if (ev->done && ev->status == "verify-match")
std::println("verified fid {} for uid {}", ev->fid, g_claim.uid); std::println("verified fid {} for uid {}", ev->fid, g_claim.uid);
break; break;
case Event::Kind::OpFinished:
g_claim.op = Op::None;
g_claim.finger.clear();
break;
} }
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
}, ev.release()); }, ev.release());
@ -1825,7 +1819,14 @@ void HandleDevice(GDBusMethodInvocation* inv, std::string_view method, GVariant*
ReturnError(inv, "NoActionInProgress", "no such operation in progress"); ReturnError(inv, "NoActionInProgress", "no such operation in progress");
return; return;
} }
// The operation is over when the CLIENT says so. A `done` status only
// means no more status is coming; fprintd's clients call Stop after
// it, and clearing the op ourselves on `done` made every one of them
// fail with NoActionInProgress. Cancelling a loop that already ended
// is harmless.
g_worker->CancelOp(); g_worker->CancelOp();
g_claim.op = Op::None;
g_claim.finger.clear();
g_dbus_method_invocation_return_value(inv, nullptr); g_dbus_method_invocation_return_value(inv, nullptr);
return; return;
} }