Bug 1697503 - Remove GeckoAppShell.KillAnyZombies. r=mossop,droeh

Before this patch, when the profile is locked on Android, we would call "ps",
parse the human-readable output and kill any other Gecko process that we could
find.

But this is completely unnecessary, as we know exactly that the PID of the
process holding the lock is.

In this patch we just kill the process holding the lock since this is
equivalent to the previous behavior.

Differential Revision: https://phabricator.services.mozilla.com/D106186
This commit is contained in:
Agi Sferro 2021-03-15 17:04:41 +00:00
parent 31aa064dcb
commit 0ae24ebfa4
10 changed files with 304 additions and 106 deletions

View File

@ -43,6 +43,10 @@
android:exported="false"
android:process=":gecko">
</service>
<!-- This is needed for ProfileLockedTest -->
<service android:name=".TestProfileLockService$p0" android:enabled="true" android:exported="false" android:process=":profiletest0" />
<service android:name=".TestProfileLockService$p1" android:enabled="true" android:exported="false" android:process=":profiletest1" />
</application>
</manifest>

View File

@ -0,0 +1,115 @@
package org.mozilla.geckoview.test
import android.content.ComponentName
import android.content.Context
import android.content.Intent
import android.content.ServiceConnection
import android.os.*
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.MediumTest
import androidx.test.platform.app.InstrumentationRegistry
import org.hamcrest.CoreMatchers.equalTo
import org.junit.Assert.assertThat
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.mozilla.geckoview.GeckoResult
import java.io.File
@RunWith(AndroidJUnit4::class)
@MediumTest
class ProfileLockedTest {
private val targetContext
get() = InstrumentationRegistry.getInstrumentation().targetContext
@get:Rule
var folder = TemporaryFolder()
/**
* Starts GeckoRuntime in the process given in input, and waits for the MESSAGE_PAGE_STOP
* event that's fired when the first GeckoSession receives the onPageStop event.
*
* We wait for a page load to make sure that everything started up correctly (as opposed
* to quitting during the startup procedure).
*/
class RuntimeInstance<T>(
private val context: Context,
private val service: Class<T>,
private val profileFolder: File) : ServiceConnection {
var isConnected = false
var disconnected = GeckoResult<Void>()
var started = GeckoResult<Void>()
var quitted = GeckoResult<Void>()
override fun onServiceConnected(name: ComponentName, binder: IBinder) {
val handler = object : Handler(Looper.getMainLooper()) {
override fun handleMessage(msg: Message) {
when (msg.what) {
TestProfileLockService.MESSAGE_PAGE_STOP -> started.complete(null)
TestProfileLockService.MESSAGE_QUIT -> {
quitted.complete(null)
// No reason to keep the service around anymore
context.unbindService(this@RuntimeInstance)
}
}
}
}
val messenger = Messenger(handler)
val service = Messenger(binder)
val msg = Message.obtain(null, TestProfileLockService.MESSAGE_REGISTER)
msg.replyTo = messenger
service.send(msg)
isConnected = true
}
override fun onServiceDisconnected(name: ComponentName?) {
isConnected = false
context.unbindService(this)
disconnected.complete(null)
}
fun start() {
val intent = Intent(context, service)
intent.putExtra("args", "-profile ${profileFolder.absolutePath}")
context.bindService(intent, this, Context.BIND_AUTO_CREATE)
}
}
@Test
fun profileLocked() {
val profileFolder = folder.newFolder("profile-locked-test")
val runtime0 = RuntimeInstance(targetContext,
TestProfileLockService.p0::class.java,
profileFolder)
val runtime1 = RuntimeInstance(targetContext,
TestProfileLockService.p1::class.java,
profileFolder)
// Start the first runtime and wait until it's ready
runtime0.start()
runtime0.started.pollDefault()
assertThat("The service should be connected now", runtime0.isConnected, equalTo(true))
// Now start a _second_ runtime with the same profile folder, this will kill the first
// runtime
runtime1.start()
// Wait for the first runtime to disconnect
runtime0.disconnected.pollDefault()
// GeckoRuntime will quit after killing the offending process
runtime1.quitted.pollDefault()
assertThat("The service shouldn't be connected anymore",
runtime0.isConnected, equalTo(false))
}
}

View File

@ -0,0 +1,99 @@
package org.mozilla.geckoview.test;
import android.app.Service;
import android.content.Intent;
import android.os.IBinder;
import android.os.Message;
import android.os.Messenger;
import android.os.RemoteException;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.mozilla.geckoview.GeckoRuntime;
import org.mozilla.geckoview.GeckoRuntimeSettings;
import org.mozilla.geckoview.GeckoSession;
public class TestProfileLockService extends Service implements
GeckoSession.ProgressDelegate,
GeckoRuntime.Delegate {
public static final class p0 extends TestProfileLockService {}
public static final class p1 extends TestProfileLockService {}
// Used by the client to register themselves
public static final int MESSAGE_REGISTER = 1;
// Sent when the first page load completes
public static final int MESSAGE_PAGE_STOP = 2;
// Sent when GeckoRuntime exits
public static final int MESSAGE_QUIT = 3;
private GeckoRuntime mRuntime;
private Messenger mClient;
private class Handler extends android.os.Handler {
@Override
public void handleMessage(@NonNull Message msg) {
switch (msg.what) {
case MESSAGE_REGISTER:
mClient = msg.replyTo;
return;
default:
throw new IllegalStateException("Unknown message: " + msg.what);
}
}
}
final Messenger mMessenger = new Messenger(new Handler());
@Override
public void onShutdown() {
final Message message = Message.obtain(null, MESSAGE_QUIT);
try {
mClient.send(message);
} catch (RemoteException ex) {
throw new RuntimeException(ex);
}
}
@Override
public void onPageStop(@NonNull GeckoSession session, boolean success) {
final Message message = Message.obtain(null, MESSAGE_PAGE_STOP);
try {
mClient.send(message);
} catch (RemoteException ex) {
throw new RuntimeException(ex);
}
}
@Override
public void onDestroy() {
// Sometimes the service doesn't die on it's own so we need to kill it here.
System.exit(0);
}
@Nullable
@Override
public IBinder onBind(Intent intent) {
// Request to be killed as soon as the client unbinds.
stopSelf();
if (mRuntime != null) {
// We only expect one client
throw new RuntimeException("Multiple clients !?");
}
GeckoRuntimeSettings settings = new GeckoRuntimeSettings.Builder()
.extras(intent.getExtras())
.build();
mRuntime = GeckoRuntime.create(getApplicationContext(), settings);
mRuntime.setDelegate(this);
final GeckoSession session = new GeckoSession();
session.setProgressDelegate(this);
session.open(mRuntime);
return mMessenger.getBinder();
}
}

View File

@ -5,10 +5,7 @@
package org.mozilla.gecko;
import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.InputStreamReader;
import java.net.Proxy;
import java.nio.ByteBuffer;
import java.util.Iterator;
@ -21,7 +18,6 @@ import org.mozilla.gecko.annotation.WrapForJNI;
import org.mozilla.gecko.util.HardwareCodecCapabilityUtils;
import org.mozilla.gecko.util.HardwareUtils;
import org.mozilla.gecko.util.InputDeviceUtils;
import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.ProxySelector;
import org.mozilla.gecko.util.ThreadUtils;
import org.mozilla.geckoview.BuildConfig;
@ -1068,94 +1064,6 @@ public class GeckoAppShell {
return result;
}
@WrapForJNI(calledFrom = "gecko")
public static void killAnyZombies() {
GeckoProcessesVisitor visitor = new GeckoProcessesVisitor() {
@Override
public boolean callback(final int pid) {
if (pid != android.os.Process.myPid())
android.os.Process.killProcess(pid);
return true;
}
};
EnumerateGeckoProcesses(visitor);
}
interface GeckoProcessesVisitor {
boolean callback(int pid);
}
private static void EnumerateGeckoProcesses(final GeckoProcessesVisitor visiter) {
int pidColumn = -1;
int userColumn = -1;
Process ps = null;
InputStreamReader inputStreamReader = null;
BufferedReader in = null;
try {
// run ps and parse its output
ps = Runtime.getRuntime().exec("ps");
inputStreamReader = new InputStreamReader(ps.getInputStream());
in = new BufferedReader(inputStreamReader, 2048);
String headerOutput = in.readLine();
// figure out the column offsets. We only care about the pid and user fields
StringTokenizer st = new StringTokenizer(headerOutput);
int tokenSoFar = 0;
while (st.hasMoreTokens()) {
String next = st.nextToken();
if (next.equalsIgnoreCase("PID"))
pidColumn = tokenSoFar;
else if (next.equalsIgnoreCase("USER"))
userColumn = tokenSoFar;
tokenSoFar++;
}
// alright, the rest are process entries.
String psOutput = null;
while ((psOutput = in.readLine()) != null) {
String[] split = psOutput.split("\\s+");
if (split.length <= pidColumn || split.length <= userColumn)
continue;
int uid = android.os.Process.getUidForName(split[userColumn]);
if (uid == android.os.Process.myUid() &&
!split[split.length - 1].equalsIgnoreCase("ps")) {
int pid = Integer.parseInt(split[pidColumn]);
boolean keepGoing = visiter.callback(pid);
if (keepGoing == false)
break;
}
}
} catch (Exception e) {
Log.w(LOGTAG, "Failed to enumerate Gecko processes.", e);
} finally {
IOUtils.safeStreamClose(in);
IOUtils.safeStreamClose(inputStreamReader);
if (ps != null) {
ps.destroy();
}
}
}
public static String getAppNameByPID(final int pid) {
BufferedReader cmdlineReader = null;
String path = "/proc/" + pid + "/cmdline";
try {
File cmdlineFile = new File(path);
if (!cmdlineFile.exists())
return "";
cmdlineReader = new BufferedReader(new FileReader(cmdlineFile));
return cmdlineReader.readLine().trim();
} catch (Exception ex) {
return "";
} finally {
IOUtils.safeStreamClose(cmdlineReader);
}
}
@WrapForJNI(calledFrom = "gecko")
private static byte[] getIconForExtension(final String aExt, final int iconSize) {
try {
@ -1451,9 +1359,6 @@ public class GeckoAppShell {
@WrapForJNI(calledFrom = "gecko")
private static boolean unlockProfile() {
// Try to kill any zombie Fennec's that might be running
GeckoAppShell.killAnyZombies();
// Then force unlock this profile
final GeckoProfile profile = GeckoThread.getActiveProfile();
if (profile != null) {

View File

@ -0,0 +1,37 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "ProfileUnlockerAndroid.h"
#include "nsPrintfCString.h"
#include <unistd.h>
namespace mozilla {
ProfileUnlockerAndroid::ProfileUnlockerAndroid(const pid_t aPid) : mPid(aPid) {}
ProfileUnlockerAndroid::~ProfileUnlockerAndroid() {}
NS_IMPL_ISUPPORTS(ProfileUnlockerAndroid, nsIProfileUnlocker)
NS_IMETHODIMP
ProfileUnlockerAndroid::Unlock(uint32_t aSeverity) {
if (aSeverity != FORCE_QUIT) {
return NS_ERROR_NOT_IMPLEMENTED;
}
NS_WARNING(nsPrintfCString("Process %d has the profile "
"lock, will try to kill it.",
mPid)
.get());
if (mPid == getpid()) {
NS_ERROR("Lock held by current process !?");
return NS_ERROR_FAILURE;
}
if (kill(mPid, SIGKILL) != 0) {
NS_WARNING("Could not kill process.");
}
return NS_OK;
}
} // namespace mozilla

View File

@ -0,0 +1,26 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef ProfileUnlockerAndroid_h
#define ProfileUnlockerAndroid_h
#include "nsIProfileUnlocker.h"
namespace mozilla {
class ProfileUnlockerAndroid final : public nsIProfileUnlocker {
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIPROFILEUNLOCKER
explicit ProfileUnlockerAndroid(const pid_t aPid);
private:
~ProfileUnlockerAndroid();
pid_t mPid;
};
} // namespace mozilla
#endif // ProfileUnlockerAndroid_h

View File

@ -25,6 +25,9 @@ UNIFIED_SOURCES += ["nsProfileLock.cpp"]
if CONFIG["OS_ARCH"] == "WINNT":
UNIFIED_SOURCES += ["ProfileUnlockerWin.cpp"]
if CONFIG["OS_TARGET"] == "Android":
UNIFIED_SOURCES += ["ProfileUnlockerAndroid.cpp"]
UNIFIED_SOURCES += ["nsToolkitProfileService.cpp"]
LOCAL_INCLUDES += [

View File

@ -7,6 +7,8 @@
#include "nsCOMPtr.h"
#include "nsQueryObject.h"
#include "nsString.h"
#include "nsPrintfCString.h"
#include "nsDebug.h"
#if defined(XP_WIN)
# include "ProfileUnlockerWin.h"
@ -17,6 +19,10 @@
# include <CoreFoundation/CoreFoundation.h>
#endif
#if defined(MOZ_WIDGET_ANDROID)
# include "ProfileUnlockerAndroid.h"
#endif
#ifdef XP_UNIX
# include <unistd.h>
# include <fcntl.h>
@ -182,7 +188,8 @@ void nsProfileLock::FatalSignalHandler(int signo
_exit(signo);
}
nsresult nsProfileLock::LockWithFcntl(nsIFile* aLockFile) {
nsresult nsProfileLock::LockWithFcntl(nsIFile* aLockFile,
nsIProfileUnlocker** aUnlocker) {
nsresult rv = NS_OK;
nsAutoCString lockFilePath;
@ -211,6 +218,14 @@ nsresult nsProfileLock::LockWithFcntl(nsIFile* aLockFile) {
mLockFileDesc = -1;
rv = NS_ERROR_FAILURE;
} else if (fcntl(mLockFileDesc, F_SETLK, &lock) == -1) {
# ifdef MOZ_WIDGET_ANDROID
MOZ_ASSERT(aUnlocker);
RefPtr<mozilla::ProfileUnlockerAndroid> unlocker(
new mozilla::ProfileUnlockerAndroid(testlock.l_pid));
nsCOMPtr<nsIProfileUnlocker> unlockerInterface(do_QueryObject(unlocker));
unlockerInterface.forget(aUnlocker);
# endif
close(mLockFileDesc);
mLockFileDesc = -1;
@ -479,7 +494,7 @@ nsresult nsProfileLock::Lock(nsIFile* aProfileDir,
// First, try locking using fcntl. It is more reliable on
// a local machine, but may not be supported by an NFS server.
rv = LockWithFcntl(lockFile);
rv = LockWithFcntl(lockFile, aUnlocker);
if (NS_SUCCEEDED(rv)) {
// Check to see whether there is a symlink lock held by an older
// Firefox build, and also place our own symlink lock --- but

View File

@ -83,7 +83,8 @@ class nsProfileLock
);
static PRCList mPidLockList;
nsresult LockWithFcntl(nsIFile* aLockFile);
nsresult LockWithFcntl(nsIFile* aLockFile,
nsIProfileUnlocker** aUnlocker = nullptr);
/**
* @param aHaveFcntlLock if true, we've already acquired an fcntl lock so this

View File

@ -2430,7 +2430,7 @@ static ReturnAbortOnError ProfileLockedDialog(nsIFile* aProfileDir,
if (aUnlocker) {
int32_t button;
#ifdef MOZ_WIDGET_ANDROID
java::GeckoAppShell::KillAnyZombies();
// On Android we always kill the process if the lock is still being held
button = 0;
#else
const uint32_t flags = (nsIPromptService::BUTTON_TITLE_IS_STRING *
@ -2457,15 +2457,8 @@ static ReturnAbortOnError ProfileLockedDialog(nsIFile* aProfileDir,
return LaunchChild(false, true);
}
} else {
#ifdef MOZ_WIDGET_ANDROID
if (java::GeckoAppShell::UnlockProfile()) {
return NS_LockProfilePath(aProfileDir, aProfileLocalDir, nullptr,
aResult);
}
#else
rv = ps->Alert(nullptr, killTitle.get(), killMessage.get());
NS_ENSURE_SUCCESS_LOG(rv, rv);
#endif
}
return NS_ERROR_ABORT;