Camera2: Handle client processes dying by closing camera resources
CameraService now subscribes to binder death notifications
for each client, and disconnects the client if the death happened
without cleanly shutting down the client first.
Bug: 7258314
Change-Id: I7803836b589fd8f0dfe00f6c28a707b82012e751
diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp
index 59ec6b6..948b59f 100644
--- a/services/camera/libcameraservice/Camera2Client.cpp
+++ b/services/camera/libcameraservice/Camera2Client.cpp
@@ -47,9 +47,10 @@
const sp<ICameraClient>& cameraClient,
int cameraId,
int cameraFacing,
- int clientPid):
+ int clientPid,
+ int servicePid):
Client(cameraService, cameraClient,
- cameraId, cameraFacing, clientPid),
+ cameraId, cameraFacing, clientPid, servicePid),
mSharedCameraClient(cameraClient),
mParameters(cameraId, cameraFacing)
{
@@ -64,10 +65,10 @@
status_t Camera2Client::checkPid(const char* checkLocation) const {
int callingPid = getCallingPid();
- if (callingPid == mClientPid) return NO_ERROR;
+ if (callingPid == mClientPid || callingPid == mServicePid) return NO_ERROR;
ALOGE("%s: attempt to use a locked camera from a different process"
- " (old pid %d, new pid %d)", checkLocation, mClientPid, callingPid);
+ " (old pid %d, new pid %d, servicePid %d)", checkLocation, mClientPid, callingPid, mServicePid);
return PERMISSION_DENIED;
}
@@ -138,8 +139,15 @@
mDestructionStarted = true;
- SharedParameters::Lock l(mParameters);
- if (l.mParameters.state != Parameters::DISCONNECTED) {
+ Parameters::State state;
+ // warning:
+ // holding on to locks more than necessary may be hazardous to your health
+ {
+ SharedParameters::Lock l(mParameters);
+ state = l.mParameters.state;
+ }
+
+ if (state != Parameters::DISCONNECTED) {
// Rewrite mClientPid to allow shutdown by CameraService
mClientPid = getCallingPid();
disconnect();
diff --git a/services/camera/libcameraservice/Camera2Client.h b/services/camera/libcameraservice/Camera2Client.h
index 3a9d307..fb1dcde 100644
--- a/services/camera/libcameraservice/Camera2Client.h
+++ b/services/camera/libcameraservice/Camera2Client.h
@@ -74,7 +74,8 @@
const sp<ICameraClient>& cameraClient,
int cameraId,
int cameraFacing,
- int clientPid);
+ int clientPid,
+ int servicePid);
virtual ~Camera2Client();
status_t initialize(camera_module_t *module);
diff --git a/services/camera/libcameraservice/Camera2Device.cpp b/services/camera/libcameraservice/Camera2Device.cpp
index 2e4098e..6da9bef 100644
--- a/services/camera/libcameraservice/Camera2Device.cpp
+++ b/services/camera/libcameraservice/Camera2Device.cpp
@@ -42,6 +42,7 @@
Camera2Device::~Camera2Device()
{
ATRACE_CALL();
+ ALOGV("%s: Tearing down for camera id %d", __FUNCTION__, mId);
disconnect();
}
diff --git a/services/camera/libcameraservice/CameraClient.cpp b/services/camera/libcameraservice/CameraClient.cpp
index c9c816a..5b59ef9 100644
--- a/services/camera/libcameraservice/CameraClient.cpp
+++ b/services/camera/libcameraservice/CameraClient.cpp
@@ -40,9 +40,9 @@
CameraClient::CameraClient(const sp<CameraService>& cameraService,
const sp<ICameraClient>& cameraClient,
- int cameraId, int cameraFacing, int clientPid):
+ int cameraId, int cameraFacing, int clientPid, int servicePid):
Client(cameraService, cameraClient,
- cameraId, cameraFacing, clientPid)
+ cameraId, cameraFacing, clientPid, servicePid)
{
int callingPid = getCallingPid();
LOG1("CameraClient::CameraClient E (pid %d, id %d)", callingPid, cameraId);
@@ -124,7 +124,7 @@
status_t CameraClient::checkPid() const {
int callingPid = getCallingPid();
- if (callingPid == mClientPid) return NO_ERROR;
+ if (callingPid == mClientPid || callingPid == mServicePid) return NO_ERROR;
ALOGW("attempt to use a locked camera from a different process"
" (old pid %d, new pid %d)", mClientPid, callingPid);
diff --git a/services/camera/libcameraservice/CameraClient.h b/services/camera/libcameraservice/CameraClient.h
index 256298d..2f31c4e 100644
--- a/services/camera/libcameraservice/CameraClient.h
+++ b/services/camera/libcameraservice/CameraClient.h
@@ -55,7 +55,8 @@
const sp<ICameraClient>& cameraClient,
int cameraId,
int cameraFacing,
- int clientPid);
+ int clientPid,
+ int servicePid);
~CameraClient();
status_t initialize(camera_module_t *module);
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 878afde..4d48d8d 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -196,11 +196,11 @@
switch(deviceVersion) {
case CAMERA_DEVICE_API_VERSION_1_0:
client = new CameraClient(this, cameraClient, cameraId,
- info.facing, callingPid);
+ info.facing, callingPid, getpid());
break;
case CAMERA_DEVICE_API_VERSION_2_0:
client = new Camera2Client(this, cameraClient, cameraId,
- info.facing, callingPid);
+ info.facing, callingPid, getpid());
break;
default:
ALOGE("Unknown camera device HAL version: %d", deviceVersion);
@@ -211,8 +211,10 @@
return NULL;
}
+ cameraClient->asBinder()->linkToDeath(this);
+
mClient[cameraId] = client;
- LOG1("CameraService::connect X (id %d)", cameraId);
+ LOG1("CameraService::connect X (id %d, this pid is %d)", cameraId, getpid());
return client;
}
@@ -220,12 +222,29 @@
int callingPid = getCallingPid();
LOG1("CameraService::removeClient E (pid %d)", callingPid);
- for (int i = 0; i < mNumberOfCameras; i++) {
- // Declare this before the lock to make absolutely sure the
- // destructor won't be called with the lock held.
- sp<Client> client;
+ // Declare this before the lock to make absolutely sure the
+ // destructor won't be called with the lock held.
+ Mutex::Autolock lock(mServiceLock);
- Mutex::Autolock lock(mServiceLock);
+ int outIndex;
+ sp<Client> client = findClientUnsafe(cameraClient, outIndex);
+
+ if (client != 0) {
+ // Found our camera, clear and leave.
+ LOG1("removeClient: clear camera %d", outIndex);
+ mClient[outIndex].clear();
+
+ client->unlinkToDeath(this);
+ }
+
+ LOG1("CameraService::removeClient X (pid %d)", callingPid);
+}
+
+sp<CameraService::Client> CameraService::findClientUnsafe(
+ const sp<ICameraClient>& cameraClient, int& outIndex) {
+ sp<Client> client;
+
+ for (int i = 0; i < mNumberOfCameras; i++) {
// This happens when we have already disconnected (or this is
// just another unused camera).
@@ -235,20 +254,21 @@
// Client::~Client() -> disconnect() -> removeClient().
client = mClient[i].promote();
- if (client == 0) {
+ // Clean up stale client entry
+ if (client == NULL) {
mClient[i].clear();
continue;
}
if (cameraClient->asBinder() == client->getCameraClient()->asBinder()) {
- // Found our camera, clear and leave.
- LOG1("removeClient: clear camera %d", i);
- mClient[i].clear();
- break;
+ // Found our camera
+ outIndex = i;
+ return client;
}
}
- LOG1("CameraService::removeClient X (pid %d)", callingPid);
+ outIndex = -1;
+ return NULL;
}
CameraService::Client* CameraService::getClientByIdUnsafe(int cameraId) {
@@ -261,6 +281,21 @@
return &mClientLock[cameraId];
}
+/*virtual*/sp<CameraService::Client> CameraService::getClientByRemote(
+ const sp<ICameraClient>& cameraClient) {
+
+ // Declare this before the lock to make absolutely sure the
+ // destructor won't be called with the lock held.
+ sp<Client> client;
+
+ Mutex::Autolock lock(mServiceLock);
+
+ int outIndex;
+ client = findClientUnsafe(cameraClient, outIndex);
+
+ return client;
+}
+
status_t CameraService::onTransact(
uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) {
// Permission checks
@@ -292,10 +327,14 @@
// the hardware first.
void CameraService::setCameraBusy(int cameraId) {
android_atomic_write(1, &mBusy[cameraId]);
+
+ ALOGV("setCameraBusy cameraId=%d", cameraId);
}
void CameraService::setCameraFree(int cameraId) {
android_atomic_write(0, &mBusy[cameraId]);
+
+ ALOGV("setCameraFree cameraId=%d", cameraId);
}
// We share the media players for shutter and recording sound for all clients.
@@ -350,7 +389,7 @@
CameraService::Client::Client(const sp<CameraService>& cameraService,
const sp<ICameraClient>& cameraClient,
- int cameraId, int cameraFacing, int clientPid) {
+ int cameraId, int cameraFacing, int clientPid, int servicePid) {
int callingPid = getCallingPid();
LOG1("Client::Client E (pid %d, id %d)", callingPid, cameraId);
@@ -359,6 +398,7 @@
mCameraId = cameraId;
mCameraFacing = cameraFacing;
mClientPid = clientPid;
+ mServicePid = servicePid;
mDestructionStarted = false;
cameraService->setCameraBusy(cameraId);
@@ -514,4 +554,26 @@
return NO_ERROR;
}
+/*virtual*/void CameraService::binderDied(
+ const wp<IBinder> &who) {
+
+ ALOGV("java clients' binder died");
+
+ sp<IBinder> whoStrong = who.promote();
+
+ if (whoStrong == 0) {
+ ALOGV("java clients' binder death already cleaned up (normal case)");
+ return;
+ }
+
+ sp<ICameraClient> iCamClient = interface_cast<ICameraClient>(whoStrong);
+
+ sp<Client> cameraClient = getClientByRemote(iCamClient);
+ ALOGW("Disconnecting camera client %p since the binder for it "
+ "died (this pid %d)", cameraClient.get(), getCallingPid());
+
+ cameraClient->disconnect();
+
+}
+
}; // namespace android
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 630fca7..f1e7df6 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -34,7 +34,8 @@
class CameraService :
public BinderService<CameraService>,
- public BnCameraService
+ public BnCameraService,
+ public IBinder::DeathRecipient
{
friend class BinderService<CameraService>;
public:
@@ -54,6 +55,8 @@
virtual Client* getClientByIdUnsafe(int cameraId);
virtual Mutex* getClientLockById(int cameraId);
+ virtual sp<Client> getClientByRemote(const sp<ICameraClient>& cameraClient);
+
virtual status_t dump(int fd, const Vector<String16>& args);
virtual status_t onTransact(uint32_t code, const Parcel& data,
Parcel* reply, uint32_t flags);
@@ -100,7 +103,8 @@
const sp<ICameraClient>& cameraClient,
int cameraId,
int cameraFacing,
- int clientPid);
+ int clientPid,
+ int servicePid);
~Client();
// return our camera client
@@ -128,6 +132,7 @@
int mCameraId; // immutable after constructor
int mCameraFacing; // immutable after constructor
pid_t mClientPid;
+ pid_t mServicePid; // immutable after constructor
};
@@ -137,6 +142,9 @@
Mutex mClientLock[MAX_CAMERAS]; // prevent Client destruction inside callbacks
int mNumberOfCameras;
+ // needs to be called with mServiceLock held
+ sp<Client> findClientUnsafe(const sp<ICameraClient>& cameraClient, int& outIndex);
+
// atomics to record whether the hardware is allocated to some client.
volatile int32_t mBusy[MAX_CAMERAS];
void setCameraBusy(int cameraId);
@@ -150,6 +158,9 @@
int mSoundRef; // reference count (release all MediaPlayer when 0)
camera_module_t *mModule;
+
+ // IBinder::DeathRecipient implementation
+ virtual void binderDied(const wp<IBinder> &who);
};
} // namespace android
diff --git a/services/camera/libcameraservice/camera2/CallbackProcessor.cpp b/services/camera/libcameraservice/camera2/CallbackProcessor.cpp
index ede97a6..3e9c255 100644
--- a/services/camera/libcameraservice/camera2/CallbackProcessor.cpp
+++ b/services/camera/libcameraservice/camera2/CallbackProcessor.cpp
@@ -86,6 +86,8 @@
// Since size should only change while preview is not running,
// assuming that all existing use of old callback stream is
// completed.
+ ALOGV("%s: Camera %d: Deleting stream %d since the buffer dimensions changed",
+ __FUNCTION__, client->getCameraId(), mCallbackStreamId);
res = device->deleteStream(mCallbackStreamId);
if (res != OK) {
ALOGE("%s: Camera %d: Unable to delete old output stream "
diff --git a/services/camera/libcameraservice/camera2/JpegProcessor.cpp b/services/camera/libcameraservice/camera2/JpegProcessor.cpp
index 7b368fa..a353679 100644
--- a/services/camera/libcameraservice/camera2/JpegProcessor.cpp
+++ b/services/camera/libcameraservice/camera2/JpegProcessor.cpp
@@ -107,6 +107,8 @@
}
if (currentWidth != (uint32_t)params.pictureWidth ||
currentHeight != (uint32_t)params.pictureHeight) {
+ ALOGV("%s: Camera %d: Deleting stream %d since the buffer dimensions changed",
+ __FUNCTION__, client->getCameraId(), mCaptureStreamId);
res = device->deleteStream(mCaptureStreamId);
if (res != OK) {
ALOGE("%s: Camera %d: Unable to delete old output stream "
diff --git a/services/camera/libcameraservice/camera2/StreamingProcessor.cpp b/services/camera/libcameraservice/camera2/StreamingProcessor.cpp
index feaeb6c..744b7ed 100644
--- a/services/camera/libcameraservice/camera2/StreamingProcessor.cpp
+++ b/services/camera/libcameraservice/camera2/StreamingProcessor.cpp
@@ -170,6 +170,9 @@
if (client == 0) return INVALID_OPERATION;
sp<Camera2Device> device = client->getCameraDevice();
+ ALOGV("%s: for cameraId %d on streamId %d",
+ __FUNCTION__, client->getCameraId(), mPreviewStreamId);
+
res = device->waitUntilDrained();
if (res != OK) {
ALOGE("%s: Error waiting for preview to drain: %s (%d)",
diff --git a/services/camera/libcameraservice/camera2/ZslProcessor.cpp b/services/camera/libcameraservice/camera2/ZslProcessor.cpp
index f462527..1bcf97e 100644
--- a/services/camera/libcameraservice/camera2/ZslProcessor.cpp
+++ b/services/camera/libcameraservice/camera2/ZslProcessor.cpp
@@ -147,6 +147,8 @@
client->getCameraId(), strerror(-res), res);
return res;
}
+ ALOGV("%s: Camera %d: Deleting stream %d since the buffer dimensions changed",
+ __FUNCTION__, client->getCameraId(), mZslStreamId);
res = device->deleteStream(mZslStreamId);
if (res != OK) {
ALOGE("%s: Camera %d: Unable to delete old output stream "