Commit 261d1107 authored by Bo Liu's avatar Bo Liu Committed by Commit Bot

android: Add method to kill child process

Add a kill method to ChildProcessConnection to force kill the service.
Also add a new field that keeps track of whether the process is killed
intentionally. Add a junit test to verify this.

Bug: 693484
Change-Id: Iaae9e82e611be97b9694fc8d67b23ba090220840
Reviewed-on: https://chromium-review.googlesource.com/1003635Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarJay Civelli <jcivelli@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550744}
parent 800802bb
...@@ -215,11 +215,14 @@ public class ChildProcessConnection { ...@@ -215,11 +215,14 @@ public class ChildProcessConnection {
// Indicates whether the connection only has the waived binding (if the connection is unbound, // Indicates whether the connection only has the waived binding (if the connection is unbound,
// it contains the state at time of unbinding). // it contains the state at time of unbinding).
private boolean mWaivedBoundOnly; private volatile boolean mWaivedBoundOnly;
// Set to true once unbind() was called. // Set to true once unbind() was called.
private boolean mUnbound; private boolean mUnbound;
// Indicate |kill()| was called to intentionally kill this process.
private volatile boolean mKilledByUs;
public ChildProcessConnection(Context context, ComponentName serviceName, boolean bindToCaller, public ChildProcessConnection(Context context, ComponentName serviceName, boolean bindToCaller,
boolean bindAsExternalService, Bundle serviceBundle) { boolean bindAsExternalService, Bundle serviceBundle) {
this(context, serviceName, bindToCaller, bindAsExternalService, serviceBundle, this(context, serviceName, bindToCaller, bindAsExternalService, serviceBundle,
...@@ -380,6 +383,19 @@ public class ChildProcessConnection { ...@@ -380,6 +383,19 @@ public class ChildProcessConnection {
notifyChildProcessDied(); notifyChildProcessDied();
} }
public void kill() {
assert isRunningOnLauncherThread();
IChildProcessService service = mService;
unbind();
try {
if (service != null) service.forceKill();
} catch (RemoteException e) {
// Intentionally ignore since we are killing it anyway.
}
mKilledByUs = true;
notifyChildProcessDied();
}
private void onServiceConnectedOnLauncherThread(IBinder service) { private void onServiceConnectedOnLauncherThread(IBinder service) {
assert isRunningOnLauncherThread(); assert isRunningOnLauncherThread();
// A flag from the parent class ensures we run the post-connection logic only once // A flag from the parent class ensures we run the post-connection logic only once
...@@ -606,6 +622,16 @@ public class ChildProcessConnection { ...@@ -606,6 +622,16 @@ public class ChildProcessConnection {
return mWaivedBoundOnly; return mWaivedBoundOnly;
} }
/**
* @return true if the connection is intentionally killed by calling kill().
*/
public boolean isKilledByUs() {
// WARNING: this method can be called from a thread other than the launcher thread.
// Note that it returns the current waived bound only state and is racy. This not really
// preventable without changing the caller's API, short of blocking.
return mKilledByUs;
}
// Should be called every time the mInitialBinding or mStrongBinding are bound/unbound. // Should be called every time the mInitialBinding or mStrongBinding are bound/unbound.
private void updateWaivedBoundOnlyState() { private void updateWaivedBoundOnlyState() {
if (!mUnbound) { if (!mUnbound) {
...@@ -629,7 +655,7 @@ public class ChildProcessConnection { ...@@ -629,7 +655,7 @@ public class ChildProcessConnection {
@VisibleForTesting @VisibleForTesting
public void crashServiceForTesting() throws RemoteException { public void crashServiceForTesting() throws RemoteException {
mService.crashIntentionallyForTesting(); mService.forceKill();
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -125,7 +125,7 @@ public abstract class ChildProcessService extends Service { ...@@ -125,7 +125,7 @@ public abstract class ChildProcessService extends Service {
} }
@Override @Override
public void crashIntentionallyForTesting() { public void forceKill() {
assert mServiceBound; assert mServiceBound;
Process.killProcess(Process.myPid()); Process.killProcess(Process.myPid());
} }
......
...@@ -18,6 +18,6 @@ interface IChildProcessService { ...@@ -18,6 +18,6 @@ interface IChildProcessService {
oneway void setupConnection(in Bundle args, ICallbackInt pidCallback, oneway void setupConnection(in Bundle args, ICallbackInt pidCallback,
in List<IBinder> clientInterfaces); in List<IBinder> clientInterfaces);
// Asks the child service to crash so that we can test the termination logic. // Forcefully kills the child process.
oneway void crashIntentionallyForTesting(); oneway void forceKill();
} }
...@@ -322,4 +322,29 @@ public class ChildProcessConnectionTest { ...@@ -322,4 +322,29 @@ public class ChildProcessConnectionTest {
mConnectionPidCallback.call(34 /* pid */); mConnectionPidCallback.call(34 /* pid */);
verify(mConnectionCallback, times(1)).onConnected(connection); verify(mConnectionCallback, times(1)).onConnected(connection);
} }
@Test
public void testKill() throws RemoteException {
ChildProcessConnection connection = createDefaultTestConnection();
assertNotNull(mFirstServiceConnection);
connection.start(false /* useStrongBinding */, null /* serviceCallback */);
mFirstServiceConnection.notifyServiceConnected(mChildProcessServiceBinder);
connection.setupConnection(
null /* connectionBundle */, null /* callback */, mConnectionCallback);
verify(mConnectionCallback, never()).onConnected(any());
ShadowLooper.runUiThreadTasks();
assertNotNull(mConnectionPidCallback);
mConnectionPidCallback.call(34 /* pid */);
verify(mConnectionCallback, times(1)).onConnected(connection);
// Add strong binding so that connection is oom protected.
connection.addStrongBinding();
Assert.assertFalse(connection.isWaivedBoundOnlyOrWasWhenDied());
// Kill and verify state.
connection.kill();
verify(mIChildProcessService).forceKill();
Assert.assertFalse(connection.isWaivedBoundOnlyOrWasWhenDied());
Assert.assertTrue(connection.isKilledByUs());
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment