[PR #2981] fix: windows connection stability #3039

New issue

Open

opened 2026-10-14 21:07:04 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-14 21:07:04 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amnezia-client/pull/2981
Author: @NickVs2015
Created: 8/12/2026
Status: 🔄 Open

Base: dev ← Head: fix/windows-connection-stability

📝 Commits (10+)

a8f3f04 fix: correct ControlService call and ANSI error message decoding on Windows
354d0b7 fix: make WireguardProtocol::stop() idempotent to avoid double deactivate()
df16abf fix: correct leak-detector dtor macro and honor service poll timeout on Windows
f581ab8 fix: detect real sleep/resume via PBT_APMSUSPEND+PBT_APMRESUMEAUTOMATIC on Windows
15de902 fix: QRingBuffer corruption under concurrent logging
fb17015 fix: recover from stuck busy-indicator/disabled-controls on real OS wakeup (Windows/Linux)
ad97f6f fix: add log systemWoke received
2395c2d fix: resolve deadlock filewrite and remove qml ApplicationActive event
ca70568 fix: retry WinTun adapter creation race after Windows resume
b933ced fix: share double-stop guard via VpnProtocol base class

📊 Changes

16 files changed (+266 additions, -31 deletions)

View changed files

📝 client/core/protocols/openVpnProtocol.cpp (+6 -0)
📝 client/core/protocols/vpnProtocol.cpp (+14 -0)
📝 client/core/protocols/vpnProtocol.h (+5 -0)
📝 client/core/protocols/wireGuardProtocol.cpp (+4 -1)
📝 client/core/protocols/xrayProtocol.cpp (+6 -0)
📝 client/platforms/windows/daemon/windowstunnelservice.cpp (+6 -1)
📝 client/platforms/windows/windowsnetworkwatcher.cpp (+6 -1)
📝 client/platforms/windows/windowsnetworkwatcher.h (+1 -0)
📝 client/platforms/windows/windowspingsender.cpp (+2 -2)
📝 client/platforms/windows/windowsservicemanager.cpp (+11 -4)
📝 client/platforms/windows/windowsutils.cpp (+4 -8)
📝 client/platforms/windows/windowsutils.h (+0 -1)
📝 client/vpnConnection.cpp (+160 -9)
📝 client/vpnConnection.h (+24 -1)
📝 common/logger/logger.cpp (+15 -3)
📝 common/logger/logger.h (+2 -0)

📄 Description

Detect real sleep/resume via PBT_APMSUSPEND+PBT_APMRESUMEAUTOMATIC on Windows** (windowsnetworkwatcher.cpp/.h)
PBT_APMRESUMESUSPEND is deprecated since Windows 8 and isn't reliably sent under Modern Standby. Switched to tracking a PBT_APMSUSPEND → PBT_APMRESUMEAUTOMATIC pairing before emitting wakeup(), so the signal fires on every real resume instead of only user-input-triggered ones, and doesn't fire spuriously. This alone cut reconnect-trigger bursts after sleep from ~6 down to ~1-2 per wake event.
Make WireguardProtocol::stop() idempotent** (wireGuardProtocol.cpp/.h)
VpnConnection::disconnectFromVpn() called m_vpnProtocol->stop() explicitly and then reset the QSharedPointer, which immediately ran the destructor — and the destructor unconditionally called stop() again, causing deactivate() to run twice per disconnect. Added an m_stopped guard so a second stop() call is a no-op.
Correct leak-detector dtor macro and honor service poll timeout on Windows** (windowstunnelservice.cpp, windowsservicemanager.cpp)
WindowsTunnelService's destructor called MZ_COUNT_CTOR instead of MZ_COUNT_DTOR (copy-paste typo), so the leak tracker's bookkeeping for that pointer never got cleared correctly — eventually tripping Q_ASSERT("leak.contains(ptr)") and aborting the debug build. Also made WindowsServiceManager::startService()/stopService() return the actual result of polling for the target service state instead of just whether the initial StartService/ControlService call was accepted, so a timed-out wait is reported as failure instead of a false success.
Correct ControlService call and ANSI error message decoding on Windows** (windowsservicemanager.cpp, windowsutils.cpp)
ControlService(..., NULL) is invalid per the Win32 API contract (the third parameter must point to a real SERVICE_STATUS) and deterministically failed with ERROR_INVALID_ADDRESS. Also fixed WindowsUtils::getErrorMessage(), which decoded FormatMessageA's ANSI-codepage output as UTF-8, turning Russian-locale error text into mojibake; now uses QString::fromLocal8Bit and includes the numeric error code in the log line.
Fix QRingBuffer corruption under concurrent logging** (common/logger/logger.cpp/.h)
The shared Logger writes to a single process-wide static QFile/QTextStream with no synchronization. tunnel.dll spins up dozens of WireGuard worker threads that log concurrently with the main thread, and unsynchronized writes to the same QFile from multiple threads corrupted Qt's internal write buffer, tripping Q_ASSERT("bytes <= bufferSize") in qringbuffer.cpp and crashing the service — most reproducible on the very first connect (cold tunnel bring-up = heaviest concurrent logging). Added a QMutex guarding every access to the shared file/stream (LogStreamer destructor, init, deInit, clearLogs, getLogFile/getServiceLogFile).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amnezia-client/pull/2981 **Author:** [@NickVs2015](https://github.com/NickVs2015) **Created:** 8/12/2026 **Status:** 🔄 Open **Base:** `dev` ← **Head:** `fix/windows-connection-stability` --- ### 📝 Commits (10+) - [`a8f3f04`](https://github.com/amnezia-vpn/amnezia-client/commit/a8f3f046d1a205e210aaeacbff7cd6c561d8043b) fix: correct ControlService call and ANSI error message decoding on Windows - [`354d0b7`](https://github.com/amnezia-vpn/amnezia-client/commit/354d0b7210205b6b8508c5c843fa086f65d0ff42) fix: make WireguardProtocol::stop() idempotent to avoid double deactivate() - [`df16abf`](https://github.com/amnezia-vpn/amnezia-client/commit/df16abf77e94cf8abdebfcd94220c1a32bd3949f) fix: correct leak-detector dtor macro and honor service poll timeout on Windows - [`f581ab8`](https://github.com/amnezia-vpn/amnezia-client/commit/f581ab8e6e68c715b31fc1fe9ae9c6182b91eae1) fix: detect real sleep/resume via PBT_APMSUSPEND+PBT_APMRESUMEAUTOMATIC on Windows - [`15de902`](https://github.com/amnezia-vpn/amnezia-client/commit/15de902111732eb19208b020b5007aee4c69b1a4) fix: QRingBuffer corruption under concurrent logging - [`fb17015`](https://github.com/amnezia-vpn/amnezia-client/commit/fb17015d21ac12364542db4d817768a0bc2bc006) fix: recover from stuck busy-indicator/disabled-controls on real OS wakeup (Windows/Linux) - [`ad97f6f`](https://github.com/amnezia-vpn/amnezia-client/commit/ad97f6f2094f4bbe06dd471ec6ac895acba70230) fix: add log systemWoke received - [`2395c2d`](https://github.com/amnezia-vpn/amnezia-client/commit/2395c2dbc51403db5e646af3fc56069bc1dbb99e) fix: resolve deadlock filewrite and remove qml ApplicationActive event - [`ca70568`](https://github.com/amnezia-vpn/amnezia-client/commit/ca7056868ae39c79861e8361dc350ef1299fdebc) fix: retry WinTun adapter creation race after Windows resume - [`b933ced`](https://github.com/amnezia-vpn/amnezia-client/commit/b933ceda664894f51d737a31e406748044cbfdf9) fix: share double-stop guard via VpnProtocol base class ### 📊 Changes **16 files changed** (+266 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `client/core/protocols/openVpnProtocol.cpp` (+6 -0) 📝 `client/core/protocols/vpnProtocol.cpp` (+14 -0) 📝 `client/core/protocols/vpnProtocol.h` (+5 -0) 📝 `client/core/protocols/wireGuardProtocol.cpp` (+4 -1) 📝 `client/core/protocols/xrayProtocol.cpp` (+6 -0) 📝 `client/platforms/windows/daemon/windowstunnelservice.cpp` (+6 -1) 📝 `client/platforms/windows/windowsnetworkwatcher.cpp` (+6 -1) 📝 `client/platforms/windows/windowsnetworkwatcher.h` (+1 -0) 📝 `client/platforms/windows/windowspingsender.cpp` (+2 -2) 📝 `client/platforms/windows/windowsservicemanager.cpp` (+11 -4) 📝 `client/platforms/windows/windowsutils.cpp` (+4 -8) 📝 `client/platforms/windows/windowsutils.h` (+0 -1) 📝 `client/vpnConnection.cpp` (+160 -9) 📝 `client/vpnConnection.h` (+24 -1) 📝 `common/logger/logger.cpp` (+15 -3) 📝 `common/logger/logger.h` (+2 -0) </details> ### 📄 Description - Detect real sleep/resume via `PBT_APMSUSPEND`+`PBT_APMRESUMEAUTOMATIC` on Windows** (`windowsnetworkwatcher.cpp/.h`) `PBT_APMRESUMESUSPEND` is deprecated since Windows 8 and isn't reliably sent under Modern Standby. Switched to tracking a `PBT_APMSUSPEND` → `PBT_APMRESUMEAUTOMATIC` pairing before emitting `wakeup()`, so the signal fires on every real resume instead of only user-input-triggered ones, and doesn't fire spuriously. This alone cut reconnect-trigger bursts after sleep from ~6 down to ~1-2 per wake event. - Make `WireguardProtocol::stop()` idempotent** (`wireGuardProtocol.cpp/.h`) `VpnConnection::disconnectFromVpn()` called `m_vpnProtocol->stop()` explicitly and then reset the `QSharedPointer`, which immediately ran the destructor — and the destructor unconditionally called `stop()` again, causing `deactivate()` to run twice per disconnect. Added an `m_stopped` guard so a second `stop()` call is a no-op. - Correct leak-detector dtor macro and honor service poll timeout on Windows** (`windowstunnelservice.cpp`, `windowsservicemanager.cpp`) `WindowsTunnelService`'s destructor called `MZ_COUNT_CTOR` instead of `MZ_COUNT_DTOR` (copy-paste typo), so the leak tracker's bookkeeping for that pointer never got cleared correctly — eventually tripping `Q_ASSERT("leak.contains(ptr)")` and aborting the debug build. Also made `WindowsServiceManager::startService()`/`stopService()` return the actual result of polling for the target service state instead of just whether the initial `StartService`/`ControlService` call was accepted, so a timed-out wait is reported as failure instead of a false success. - Correct `ControlService` call and ANSI error message decoding on Windows** (`windowsservicemanager.cpp`, `windowsutils.cpp`) `ControlService(..., NULL)` is invalid per the Win32 API contract (the third parameter must point to a real `SERVICE_STATUS`) and deterministically failed with `ERROR_INVALID_ADDRESS`. Also fixed `WindowsUtils::getErrorMessage()`, which decoded `FormatMessageA`'s ANSI-codepage output as UTF-8, turning Russian-locale error text into mojibake; now uses `QString::fromLocal8Bit` and includes the numeric error code in the log line. - Fix `QRingBuffer` corruption under concurrent logging** (`common/logger/logger.cpp/.h`) The shared `Logger` writes to a single process-wide static `QFile`/`QTextStream` with no synchronization. `tunnel.dll` spins up dozens of WireGuard worker threads that log concurrently with the main thread, and unsynchronized writes to the same `QFile` from multiple threads corrupted Qt's internal write buffer, tripping `Q_ASSERT("bytes <= bufferSize")` in `qringbuffer.cpp` and crashing the service — most reproducible on the very first connect (cold tunnel bring-up = heaviest concurrent logging). Added a `QMutex` guarding every access to the shared file/stream (`LogStreamer` destructor, `init`, `deInit`, `clearLogs`, `getLogFile`/`getServiceLogFile`). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-14 21:07:04 +00:00

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

amnezia-vpn/amnezia-client#3039

No description provided.

Rows
Columns