diff --git a/lib/devices.ts b/lib/devices.ts index bbfce0b..a7ed7f8 100644 --- a/lib/devices.ts +++ b/lib/devices.ts @@ -284,42 +284,69 @@ export class Devices extends EventEmitter { state.hasCard = true; - // Try to connect to the card - try { - const readers = this._context!.listReaders(); - const reader = readers.find((r) => r.name === readerName); + // Resolve reader with a short retry window for transient list churn. + const maxLookupAttempts = 3; + const lookupRetryDelayMs = 50; + let reader: Reader | undefined; + + for (let attempt = 0; attempt < maxLookupAttempts; attempt++) { + if (!this._running || !this._context || !this._context.isValid) { + return; + } + const readers = this._context.listReaders(); + reader = readers.find((r) => r.name === readerName); if (reader) { - let nativeCard: Card; - try { - // First try with both T=0 and T=1 protocols + break; + } + + if (attempt < maxLookupAttempts - 1) { + await new Promise((resolve) => + setTimeout(resolve, lookupRetryDelayMs) + ); + } + } + + if (!reader) { + this.emit( + 'error', + new Error( + `Reader not found while handling card insertion: ${readerName}` + ) + ); + return; + } + + // Try to connect to the card + try { + let nativeCard: Card; + try { + // First try with both T=0 and T=1 protocols + nativeCard = await reader.connect( + this._SCARD_SHARE_SHARED, + this._SCARD_PROTOCOL_T0 | this._SCARD_PROTOCOL_T1 + ); + } catch (dualProtocolErr) { + // If dual protocol fails with unresponsive card error, + // fallback to T=0 only (issue #34) + if (isUnresponsiveCardError(dualProtocolErr as Error)) { nativeCard = await reader.connect( this._SCARD_SHARE_SHARED, - this._SCARD_PROTOCOL_T0 | this._SCARD_PROTOCOL_T1 + this._SCARD_PROTOCOL_T0 ); - } catch (dualProtocolErr) { - // If dual protocol fails with unresponsive card error, - // fallback to T=0 only (issue #34) - if (isUnresponsiveCardError(dualProtocolErr as Error)) { - nativeCard = await reader.connect( - this._SCARD_SHARE_SHARED, - this._SCARD_PROTOCOL_T0 - ); - } else { - // Re-throw if it's a different error - throw dualProtocolErr; - } + } else { + throw dualProtocolErr; } + } - // Wrap the native card to add autoGetResponse support - const card = wrapCard(nativeCard); - state.card = card; + // Wrap the native card to add autoGetResponse support + const card = wrapCard(nativeCard); + state.card = card; - this.emit('card-inserted', { - reader: { name: readerName, state: eventState, atr: atr }, - card: card, - }); - } + this.emit('card-inserted', { + reader: { name: readerName, state: eventState, atr: atr }, + card: card, + }); } catch (err) { // Emit error but don't fail this.emit('error', err as Error); diff --git a/src/async_workers.cpp b/src/async_workers.cpp index 8ef88d7..1d35011 100644 --- a/src/async_workers.cpp +++ b/src/async_workers.cpp @@ -60,10 +60,10 @@ void WaitForChangeWorker::OnOK() { } deferred_.Resolve(changes); - } else if (result_ == SCARD_E_CANCELLED) { + } else if (result_ == static_cast(SCARD_E_CANCELLED)) { // Cancelled - resolve with null deferred_.Resolve(env.Null()); - } else if (result_ == SCARD_E_TIMEOUT) { + } else if (result_ == static_cast(SCARD_E_TIMEOUT)) { // Timeout - resolve with empty array deferred_.Resolve(Napi::Array::New(env, 0)); } else { diff --git a/src/pcsc_context.cpp b/src/pcsc_context.cpp index 9363d51..ddf2ddd 100644 --- a/src/pcsc_context.cpp +++ b/src/pcsc_context.cpp @@ -62,7 +62,7 @@ Napi::Value PCSCContext::ListReaders(const Napi::CallbackInfo& info) { DWORD readersLen = 0; LONG result = SCardListReaders(context_, nullptr, nullptr, &readersLen); - if (result == SCARD_E_NO_READERS_AVAILABLE) { + if (result == static_cast(SCARD_E_NO_READERS_AVAILABLE)) { // No readers - return empty array return Napi::Array::New(env, 0); } @@ -164,7 +164,7 @@ Napi::Value PCSCContext::WaitForChange(const Napi::CallbackInfo& info) { DWORD readersLen = 0; LONG result = SCardListReaders(context_, nullptr, nullptr, &readersLen); - if (result == SCARD_E_NO_READERS_AVAILABLE) { + if (result == static_cast(SCARD_E_NO_READERS_AVAILABLE)) { // Add PnP notification to detect when readers are attached readerNames.push_back("\\\\?PnP?\\Notification"); currentStates.push_back(SCARD_STATE_UNAWARE); @@ -200,7 +200,7 @@ Napi::Value PCSCContext::Cancel(const Napi::CallbackInfo& info) { } LONG result = SCardCancel(context_); - if (result != SCARD_S_SUCCESS && result != SCARD_E_INVALID_HANDLE) { + if (result != SCARD_S_SUCCESS && result != static_cast(SCARD_E_INVALID_HANDLE)) { Napi::Error::New(env, GetPCSCErrorString(result)).ThrowAsJavaScriptException(); } diff --git a/src/reader_monitor.cpp b/src/reader_monitor.cpp index 96537e7..b46aca4 100644 --- a/src/reader_monitor.cpp +++ b/src/reader_monitor.cpp @@ -240,11 +240,11 @@ void ReaderMonitor::MonitorLoop() { break; } - if (result == SCARD_E_CANCELLED) { + if (result == static_cast(SCARD_E_CANCELLED)) { break; } - if (result == SCARD_E_TIMEOUT) { + if (result == static_cast(SCARD_E_TIMEOUT)) { // Timeout - query fresh state to detect missed events (Issue #111) // On Windows, dwEventState after timeout may just mirror dwCurrentState // rather than reflecting actual hardware state. We must explicitly @@ -322,6 +322,8 @@ void ReaderMonitor::MonitorLoop() { // PnP notification - reader list changed if (readerNames[i] == "\\\\?PnP?\\Notification") { pnpTriggered = true; + const auto oldReaderStates = readerStates_; + // Get old reader names std::vector oldNames; for (const auto& pair : readerStates_) { @@ -351,12 +353,33 @@ void ReaderMonitor::MonitorLoop() { EmitEvent("reader-detached", old, 0, {}); } } - continue; + + // Reconcile card presence for readers that still exist after PnP update. + // This minimizes missed insert/remove events that occurred during reader churn. + for (const auto& pair : readerStates_) { + const std::string& name = pair.first; + auto oldIt = oldReaderStates.find(name); + if (oldIt == oldReaderStates.end()) { + continue; + } + + DWORD oldState = oldIt->second.lastState; + DWORD newState = pair.second.lastState; + + bool wasPresent = (oldState & SCARD_STATE_PRESENT) != 0; + bool isPresent = (newState & SCARD_STATE_PRESENT) != 0; + + if (!wasPresent && isPresent) { + EmitEvent("card-inserted", name, newState, pair.second.atr); + } else if (wasPresent && !isPresent) { + EmitEvent("card-removed", name, newState, {}); + } + } + + // Reader indices from this event batch may now be stale. + break; } - // Skip reader state processing if PnP was triggered in this iteration - // The reader list has changed, so indices are no longer valid - // We'll pick up any card changes on the next iteration with fresh state if (pnpTriggered) { continue; } @@ -390,6 +413,10 @@ void ReaderMonitor::MonitorLoop() { } } } + + if (pnpTriggered) { + continue; + } } } @@ -398,7 +425,7 @@ void ReaderMonitor::UpdateReaderList() { DWORD readersLen = 0; LONG result = SCardListReaders(context_, nullptr, nullptr, &readersLen); - if (result == SCARD_E_NO_READERS_AVAILABLE || readersLen == 0) { + if (result == static_cast(SCARD_E_NO_READERS_AVAILABLE) || readersLen == 0) { readerStates_.clear(); return; } @@ -423,25 +450,47 @@ void ReaderMonitor::UpdateReaderList() { p += strlen(p) + 1; } - // Get initial state for new readers + // Preserve previous state for readers that still exist + const auto previousStates = readerStates_; + + // Get initial state for listed readers std::vector states(newNames.size()); for (size_t i = 0; i < newNames.size(); i++) { states[i].szReader = newNames[i].c_str(); states[i].dwCurrentState = SCARD_STATE_UNAWARE; } - SCardGetStatusChange(context_, 0, states.data(), states.size()); + LONG stateResult = SCARD_S_SUCCESS; + if (!states.empty()) { + stateResult = SCardGetStatusChange(context_, 0, states.data(), states.size()); + } // Update reader states map (Issue #111 fix: use map keyed by name) - readerStates_.clear(); + std::unordered_map updatedStates; for (size_t i = 0; i < newNames.size(); i++) { - ReaderInfo info; - info.lastState = states[i].dwEventState & ~SCARD_STATE_CHANGED; - if (states[i].cbAtr > 0) { - info.atr.assign(states[i].rgbAtr, states[i].rgbAtr + states[i].cbAtr); + const std::string& name = newNames[i]; + ReaderInfo info = {}; + + auto previousIt = previousStates.find(name); + if (previousIt != previousStates.end()) { + info = previousIt->second; + } else { + info.lastState = SCARD_STATE_UNAWARE; } - readerStates_[newNames[i]] = info; + + if (stateResult == SCARD_S_SUCCESS) { + info.lastState = states[i].dwEventState & ~SCARD_STATE_CHANGED; + if (states[i].cbAtr > 0) { + info.atr.assign(states[i].rgbAtr, states[i].rgbAtr + states[i].cbAtr); + } else { + info.atr.clear(); + } + } + + updatedStates[name] = info; } + + readerStates_.swap(updatedStates); } void ReaderMonitor::EmitEvent(const std::string& eventType, const std::string& readerName, @@ -450,9 +499,9 @@ void ReaderMonitor::EmitEvent(const std::string& eventType, const std::string& r // before the callback executes (prevents memory leak) auto data = std::make_shared(EventData{eventType, readerName, state, atr}); - // Call JavaScript callback on main thread - // Capture shared_ptr by value to extend lifetime until callback executes - tsfn_.BlockingCall(data.get(), [data](Napi::Env env, Napi::Function callback, EventData* ptr) { + // Call JavaScript callback on main thread without blocking monitor thread. + // Capture shared_ptr by value to extend lifetime until callback executes. + napi_status status = tsfn_.NonBlockingCall(data.get(), [data](Napi::Env env, Napi::Function callback, EventData* ptr) { // Build event object Napi::Object event = Napi::Object::New(env); event.Set("type", Napi::String::New(env, ptr->eventType)); @@ -469,4 +518,7 @@ void ReaderMonitor::EmitEvent(const std::string& eventType, const std::string& r callback.Call({event}); // shared_ptr automatically cleaned up when lambda is destroyed }); + + // Ignore errors when queue is closing during shutdown. + (void)status; }