Windows: Refactor COM port read error handling

[Jason2866]

The overlapped's hEvent holds a ReadBaton pointer (not a Windows event handle), so GetOverlappedResult fails with ERROR_INVALID_HANDLE on drivers that inspect hEvent (e.g. usbser.sys used by ESP32 native USB).

fix: serialport/node-serialport#3121 and serialport/node-serialport#3086

---

Why GetOverlappedResult Fails with ERROR_INVALID_HANDLE

The internal implementation of GetOverlappedResult works by calling WaitForSingleObject on po->hEvent (if it is non-NULL), or on the file handle itself if hEvent is NULL.

If bWait is TRUE, GetOverlappedResult determines whether the pending operation has been completed by waiting for the event object to be in the signaled state. If hEvent is NULL, the system uses the state of the hFile handle instead.

The problem occurs with certain third-party Windows COM drivers that inspect or manipulate the hEvent field of the OVERLAPPED structure. When using IOCP, the hEvent field is often NULL or contains a value that is not a valid event handle (e.g., it may be used by the driver internally). When an asynchronous I/O request completes, the device driver checks to see whether hEvent is NULL. If hEvent is not NULL, the driver signals the event by calling SetEvent. Drivers that deviate from this contract and store invalid values in hEvent cause GetOverlappedResult to call WaitForSingleObject on an invalid handle, resulting in ERROR_INVALID_HANDLE.

---

The Fix

When using IOCP, completions and failures should always be handled from the completion port worker thread. GetOverlappedResult is generally used with non-completion-port overlapped I/O.

In the IOCP completion callback (WriteIOCompletion), the bytesTransferred value is delivered directly as a parameter by the Windows kernel — it is the same value that would have been read from OVERLAPPED::InternalHigh by GetOverlappedResult. By removing the GetOverlappedResult call and using bytesTransferred directly from the callback parameter, the fix:

1. Eliminates the interaction with hEvent entirely, sidestepping the ERROR_INVALID_HANDLE failure on buggy/third-party drivers.
2. Is semantically correct: the IOCP completion callback is the canonical place to consume bytesTransferred — no additional call to GetOverlappedResult is needed or recommended.
3. Avoids the previous errorCode overwrite bug, where GetLastError() was called after GetOverlappedResult could have modified the last-error state.

---

The Root Cause

The design intentionally stores a WriteBaton* pointer in ov->hEvent:

// In WriteThread:
ov->hEvent = static_cast<void*>(baton);

This is explicitly allowed by MSDN for WriteFileEx/ReadFileEx — the documentation states that hEvent is not used by the system for APC-based I/O and is reserved for user data. The problem arose because the old code then called GetOverlappedResult on this same OVERLAPPED, whose hEvent holds a baton pointer — not a valid Windows event handle.

Certain drivers (notably usbser.sys, used by ESP32 native USB, and drivers for CP210x/CH340-based USB serial chips) internally call WaitForSingleObject(ov->hEvent, ...) or otherwise validate hEvent when servicing GetOverlappedResult. Since the value is a baton pointer (not a valid HANDLE), WaitForSingleObject fails immediately with ERROR_INVALID_HANDLE.

---

Why the Fix Is Correct

The new WriteIOCompletion uses bytesTransferred directly from the APC callback parameter:

void __stdcall WriteIOCompletion(DWORD errorCode, DWORD bytesTransferred, OVERLAPPED* ov) {
  WriteBaton* baton = static_cast<WriteBaton*>(ov->hEvent);

  if (errorCode) {
    ErrorCodeToString("Writing to COM port (WriteIOCompletion)", errorCode, baton->errorString);
    baton->complete = true;
    return;
  }

  if (bytesTransferred) {
    baton->offset += bytesTransferred;
    if (baton->offset >= baton->bufferLength) {
      baton->complete = true;
    }
  }
}

This is correct for three reasons:

1. bytesTransferred in the APC callback is authoritative. The Windows kernel populates this value directly from the I/O completion — it is the same value that GetOverlappedResult would have read from OVERLAPPED::InternalHigh. No information is lost.

2. MSDN explicitly forbids GetOverlappedResult here. The comment in the code correctly cites MSDN: "Do not use GetOverlappedResult for I/O operations that use ReadFileEx or WriteFileEx completion routines."

3. The fix eliminates all interaction with hEvent during the write completion path, making the code immune to any driver-side inspection of that field.

---

Consistency with ReadIOCompletion

The same fix is applied to ReadIOCompletion (same comment, same pattern). Notably, ReadIOCompletion still uses GetOverlappedResult in its fallback ReadFile path — but it correctly creates a real event handle there first:

ov->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
// ... ReadFile + GetOverlappedResult ...
CloseHandle(ov->hEvent);

This is safe because hEvent is a proper HANDLE at that point. The two code paths are now consistent and correct.

---