Skip to content

Commit fc0f39e

Browse files
pmaxhoganclaude
andcommitted
fix(power): balance windows COM init with CoUninitialize (RAII guard)
C-P2-6: the Windows metered-network detect_metered called CoInitializeEx on every 30s poll but never CoUninitialize, leaking a COM apartment refcount. Added a ComInitGuard RAII type that calls CoUninitialize on drop, armed ONLY when our CoInitializeEx returned success (S_OK or S_FALSE, both is_ok() and both owing an uninit). RPC_E_CHANGED_MODE (the thread was already initialised differently) is still tolerated and is NOT balanced (we did not perform that init); any other hard init failure aborts with no uninit owed. cfg(windows)-gated so non-Windows clippy sees no dead code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent 30e8797 commit fc0f39e

1 file changed

Lines changed: 41 additions & 8 deletions

File tree

crates/driven-power/src/network.rs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,12 @@ pub(crate) fn detect_metered_and_reachable() -> (bool, bool) {
8787
/// collapses to the safe `false` default rather than guessing "metered" -
8888
/// a wrong "metered" would stall ALL sync, a wrong "not metered" only fails
8989
/// to skip a rare metered link. COM is initialised per call as
90-
/// multi-threaded-apartment; an `RPC_E_CHANGED_MODE` (apartment already
91-
/// initialised differently on this thread by the host process) is tolerated
92-
/// because the COM call still works against the existing apartment.
90+
/// multi-threaded-apartment and BALANCED with `CoUninitialize` on scope exit
91+
/// via an RAII guard (codex C-P2-6 - the prior code leaked an apartment
92+
/// refcount on every 30s poll); an `RPC_E_CHANGED_MODE` (apartment already
93+
/// initialised differently on this thread by the host process) is tolerated -
94+
/// the COM call still works against the existing apartment - and is NOT
95+
/// balanced (we did not perform that init).
9396
#[cfg(target_os = "windows")]
9497
fn detect_metered() -> bool {
9598
use windows::core::HRESULT;
@@ -100,25 +103,55 @@ fn detect_metered() -> bool {
100103
NLM_CONNECTION_COST_VARIABLE,
101104
};
102105
use windows::Win32::System::Com::{
103-
CoCreateInstance, CoInitializeEx, CLSCTX_ALL, COINIT_MULTITHREADED,
106+
CoCreateInstance, CoInitializeEx, CoUninitialize, CLSCTX_ALL, COINIT_MULTITHREADED,
104107
};
105108

106109
// RPC_E_CHANGED_MODE: COM already initialised on this thread with a
107110
// different apartment model. Not fatal - the existing apartment serves
108111
// our in-proc call fine, so we proceed without treating it as an error.
109112
const RPC_E_CHANGED_MODE: HRESULT = HRESULT(0x8001_0106u32 as i32);
110113

114+
/// RAII guard that balances a SUCCESSFUL `CoInitializeEx` with exactly one
115+
/// `CoUninitialize` on drop (codex C-P2-6: the previous code never
116+
/// uninitialised, leaking a COM apartment refcount on every 30s poll).
117+
///
118+
/// COM rule: every `CoInitializeEx` that returns success - `S_OK` OR
119+
/// `S_FALSE` ("already initialised on this thread", still a success that
120+
/// increments the per-thread refcount) - MUST be balanced by one
121+
/// `CoUninitialize`. A `RPC_E_CHANGED_MODE` is a FAILURE (the thread was
122+
/// already initialised with a different model) and must NOT be balanced, so
123+
/// the guard is only armed when we performed a successful init.
124+
struct ComInitGuard {
125+
// `true` only when our CoInitializeEx returned success and therefore
126+
// owes a CoUninitialize.
127+
should_uninit: bool,
128+
}
129+
impl Drop for ComInitGuard {
130+
fn drop(&mut self) {
131+
if self.should_uninit {
132+
// SAFETY: balanced against our own successful CoInitializeEx on
133+
// this same thread (the guard is never moved across threads).
134+
unsafe { CoUninitialize() };
135+
}
136+
}
137+
}
138+
111139
// SAFETY: standard COM init -> create-instance -> query-facet -> call
112140
// sequence. Each pointer is owned by the `windows` smart wrappers
113-
// (refcounted), and every fallible step short-circuits to `false`.
141+
// (refcounted), every fallible step short-circuits to `false`, and the
142+
// ComInitGuard balances a successful init on every return path (drop runs
143+
// even on the early `return false`s below).
114144
unsafe {
115145
let init = CoInitializeEx(None, COINIT_MULTITHREADED);
116-
// `CoInitializeEx` returns an HRESULT-like; S_FALSE means "already
117-
// initialised on this thread" (still success). Only a hard failure
118-
// other than CHANGED_MODE should abort.
146+
// `is_ok()` covers both S_OK and S_FALSE (>= 0). Those owe an uninit.
147+
// RPC_E_CHANGED_MODE: proceed but do NOT uninit (we did not init). Any
148+
// other hard failure aborts (and owes no uninit).
119149
if init.is_err() && init != RPC_E_CHANGED_MODE {
120150
return false;
121151
}
152+
let _com_guard = ComInitGuard {
153+
should_uninit: init.is_ok(),
154+
};
122155

123156
let cost_manager: windows::core::Result<INetworkCostManager> =
124157
CoCreateInstance(&NetworkListManager, None, CLSCTX_ALL);

0 commit comments

Comments
 (0)