Skip to content

Fix #70: Add @Volatile to DefaultNetworkListener fields - #73

Open
aimkiray wants to merge 1 commit into
meow-rs:mainfrom
aimkiray:fix/network-listener-volatile
Open

Fix #70: Add @Volatile to DefaultNetworkListener fields#73
aimkiray wants to merge 1 commit into
meow-rs:mainfrom
aimkiray:fix/network-listener-volatile

Conversation

@aimkiray

Copy link
Copy Markdown

Summary

Fixes #70

Supersedes the @Volatile portion of closed PR #67, which was incompatible with main after the Compose migration (#63).

Problem

DefaultNetworkListener is a Kotlin object (singleton) with two mutable fields:

private var callback: ConnectivityManager.NetworkCallback? = null
private var connectivityManager: ConnectivityManager? = null

Without @Volatile, the Java Memory Model does not guarantee that writes made by one thread are visible to reads on another thread.

Fix

Add @Volatile to both fields:

@Volatile private var callback: ConnectivityManager.NetworkCallback? = null
@Volatile private var connectivityManager: ConnectivityManager? = null

Analysis

In the current codebase, both start() and stop() execute on Dispatchers.Main.immediate (the main thread):

  • start() is called from VpnService.preInit()GlobalScope.launch(Dispatchers.Main.immediate)
  • stop() is called from VpnService.killProcesses()GlobalScope.launch(Dispatchers.Main.immediate)scope.launch { } (inherits Main.immediate)

There is no cross-thread access to these fields. The @Volatile annotation is therefore a defensive best-practice improvement rather than a fix for an observed bug. However, it removes a latent hazard if the call sites are ever refactored to use a different dispatcher.

Note: The issue description mentions that the fields are "read on the ConnectivityManager callback (binder) thread." However, the onAvailable()/onLost()/onCapabilitiesChanged() callbacks access the onNetworkChanged lambda parameter, not the callback or connectivityManager fields themselves. These fields are only read/written in start() and stop(), both on the main thread.

Files changed

  • core/.../net/DefaultNetworkListener.kt — 2 lines changed (@Volatile added to 2 fields)

…read visibility

connectivityManager and callback are plain var fields written in
start()/stop() on Dispatchers.Main.immediate. Without @volatile there
is no happens-before guarantee that writes are visible if these fields
are ever accessed from a different dispatcher. Benign in the current
lifecycle ordering (all access is on the main thread), but removes
the latent hazard.

Fixes meow-rs#70
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] DefaultNetworkListener fields lack @Volatile for cross-thread visibility

1 participant