am ecf17e82: Camera2: Handle client processes dying by closing camera resources
* commit 'ecf17e82505fdb60d59e00b6dd59036df93de655':
Camera2: Handle client processes dying by closing camera resources
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 "