Skip to content

App crashes when tapping Disconnect button #666

@ilok67

Description

@ilok67

When the user taps the Disconnect button, the app crashes. This happens because MainActivity.onStop callback sends ACTION_STOP via startService() and then immediately calls stopService() on the same service, creating a race condition with the teardown thread inside MhrvVpnService.

Root cause:

In MainActivity, the onStop lambda does:

onStop = {
    val stopAction = Intent(this, MhrvVpnService::class.java)
        .setAction(MhrvVpnService.ACTION_STOP)
    startService(stopAction)
    stopService(Intent(this, MhrvVpnService::class.java))  // ← crashes here
}

Meanwhile in MhrvVpnService.onStartCommand, the ACTION_STOP branch spawns a background thread to run teardown() and then calls stopSelf():

ACTION_STOP -> {
    try { stopForeground(STOP_FOREGROUND_REMOVE) } catch ...
    Thread({
        teardown()
        stopSelf()
    }, "mhrv-teardown").start()
    START_NOT_STICKY
}

The problem:

  1. startService(stopAction) delivers ACTION_STOP → spawns "mhrv-teardown" thread → thread starts teardown() (which includes Native.stopProxy(handle) and can take several seconds).
  2. stopService() is called immediately after on the same service. This triggers onDestroy() while the teardown thread is still running. Two threads now race through service lifecycle — the background thread calling stopSelf() on a service that stopService() is already destroying.
  3. Depending on timing, this can cause:
    · stopSelf() crashing because the service is already being destroyed by stopService()
    · onDestroy() racing with the teardown thread mid-cleanup
    · The service process dying while native handles are still being released

Fix:

Remove the redundant stopService() call. The ACTION_STOP path in MhrvVpnService already handles the full graceful teardown (stop foreground → teardown on background thread → stopSelf). The extra stopService() is unnecessary and dangerous.

onStop = {
    val stopAction = Intent(this, MhrvVpnService::class.java)
        .setAction(MhrvVpnService.ACTION_STOP)
    startService(stopAction)
    // stopService(Intent(this, MhrvVpnService::class.java))  ← REMOVE THIS LINE
}

Steps to reproduce:

  1. Start the VPN (tap Connect)
  2. Wait for it to be fully running
  3. Tap Disconnect
  4. App crashes

Expected behavior:
App should stop the VPN gracefully without crashing.

Additional notes:
The MhrvVpnService.teardown() already has an idempotency guard (tornDown AtomicBoolean), which is correct for handling the onDestroy path. But the root issue is that stopService() shouldn't be called at all when we've already sent ACTION_STOP — let the service handle its own shutdown.


Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions