From 84ac2b06fae97318594a6884d2fcddc38dcfe7d8 Mon Sep 17 00:00:00 2001
From: Mygod <mygodstudio@gmail.com>
Date: Mon, 11 Jul 2016 19:22:54 +0800
Subject: [PATCH] Refine #530 again

Have. To. Do. More. Refinements.

Here's what happened in the buggy versions: (I think)

* `BootReceiver` starts which invokes `Utils.startSsService` which starts `ShadowsocksRunnerService`;
* `ShadowsocksRunnerService` binds to `BaseService`;
* 1s after it's connected, it invokes `bgService.use` which returns immediately after pushing `use` in the queue since it's an oneway method;
* Then `stopSelf` => `detachService`, `BaseService` now have nothing bound to it and not started either (neither foreground nor background), so the system can destroy it any time; (at this point, `BaseService.use` isn't invoked yet)
* `com.github.shadowsocks` process then probably exits since booting is a resource-heavy task, anyway it doesn't matter;
* Some time later `com.github.shadowsocks:bg` resumes running, and `BaseService` is stopped and destroyed. Now the system is no longer aware of this `BaseService` instance but it isn't garbage collected yet;
* `use` is finally invoked;
* `startService` is invoked in `startRunner` and the destroyed background service starts running; (bug of Android?)
* `startForeground` is invoked in `ShadowsocksNotification` but the system probably refused it since the service itself is already destroyed;
* When the main activity UI is brought up and requests binding, `com.github.shadowsocks:bg` which is unaware of the existing instance of `BaseService` creates another `BaseService` and thus everything gets messed up.

This commit fixes this by invoking `getState` (which isn't an oneway method) immediately after `use` so that `ShadowsocksRunnerService` will wait for `BaseService` to start itself before destroying itself and its connection to `BaseService`. And thus its return value is not used.
---
 .../shadowsocks/ShadowsocksNotification.scala |  4 +---
 .../ShadowsocksRunnerService.scala            | 24 +++++--------------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/src/main/scala/com/github/shadowsocks/ShadowsocksNotification.scala b/src/main/scala/com/github/shadowsocks/ShadowsocksNotification.scala
index 73b61938..8df9e328 100644
--- a/src/main/scala/com/github/shadowsocks/ShadowsocksNotification.scala
+++ b/src/main/scala/com/github/shadowsocks/ShadowsocksNotification.scala
@@ -83,9 +83,7 @@ class ShadowsocksNotification(private val service: BaseService, profileName: Str
     show()
   } else if (forceShow) show()
 
-  def show() = {
-    service.startForeground(1, builder.build)
-  }
+  def show() = service.startForeground(1, builder.build)
 
   def destroy() {
     if (lockReceiver != null) {
diff --git a/src/main/scala/com/github/shadowsocks/ShadowsocksRunnerService.scala b/src/main/scala/com/github/shadowsocks/ShadowsocksRunnerService.scala
index 0bb2baab..9af9c182 100644
--- a/src/main/scala/com/github/shadowsocks/ShadowsocksRunnerService.scala
+++ b/src/main/scala/com/github/shadowsocks/ShadowsocksRunnerService.scala
@@ -43,7 +43,6 @@ import android.app.Service
 import android.content.Intent
 import android.net.VpnService
 import android.os.{IBinder, Handler}
-import com.github.shadowsocks.utils.Action
 import com.github.shadowsocks.utils.ConfigUtils
 import com.github.shadowsocks.ShadowsocksApplication.app
 
@@ -55,29 +54,18 @@ class ShadowsocksRunnerService extends Service with ServiceBoundContext {
   }
 
   override def onServiceConnected() {
-    handler.postDelayed(() => if (bgService != null) startBackgroundService(), 1000)
-  }
-
-  def startBackgroundService() {
-    if (app.isNatEnabled) bgService.use(app.profileId) else {
-      val intent = VpnService.prepare(ShadowsocksRunnerService.this)
-      if (intent == null) {
-        if (bgService != null) {
-          bgService.use(app.profileId)
-        }
+    handler.postDelayed(() => if (bgService != null) {
+      if (app.isNatEnabled || VpnService.prepare(ShadowsocksRunnerService.this) == null) {
+        bgService.use(app.profileId)
+        bgService.getState  // ensure the oneway call to use is finished
       }
-    }
-    stopSelf()
+      stopSelf()
+    }, 1000)
   }
 
   override def onCreate() {
     super.onCreate()
     attachService()
-
-    val s = if (app.isNatEnabled) classOf[ShadowsocksNatService] else classOf[ShadowsocksVpnService]
-    val intent = new Intent(this, s)
-    intent.setAction(Action.SERVICE)
-    startService(intent)
   }
 
   override def onDestroy() {