diff options
author | Igor Murashkin <iam@google.com> | 2013-03-13 17:23:00 -0700 |
---|---|---|
committer | Igor Murashkin <iam@google.com> | 2013-03-13 17:23:00 -0700 |
commit | acd695c42749f8821b0a0cc27739ddf096c6d4e8 (patch) | |
tree | a235142773c93b7aaaf8b70c5f8de955d524ce3c | |
parent | f15c0d6d0d80899da9c2d0c479aebc7f42464f27 (diff) | |
download | frameworks_av-acd695c42749f8821b0a0cc27739ddf096c6d4e8.zip frameworks_av-acd695c42749f8821b0a0cc27739ddf096c6d4e8.tar.gz frameworks_av-acd695c42749f8821b0a0cc27739ddf096c6d4e8.tar.bz2 |
ProCamera: Fix rare deadlock when client destructs inside the connect call
Bug: 8337737
Change-Id: Ia6fca4365fa20fdbfd6a1ec8d047639a002f2aba
-rw-r--r-- | services/camera/libcameraservice/CameraService.cpp | 164 |
1 files changed, 87 insertions, 77 deletions
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 7636143..5a6a3c8 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -277,55 +277,61 @@ sp<ICamera> CameraService::connect( sp<Client> client; - Mutex::Autolock lock(mServiceLock); - if (!canConnectUnsafe(cameraId, clientPackageName, - cameraClient->asBinder(), - /*out*/client)) { - return NULL; - } else if (client.get() != NULL) { - return client; - } + { + Mutex::Autolock lock(mServiceLock); + if (!canConnectUnsafe(cameraId, clientPackageName, + cameraClient->asBinder(), + /*out*/client)) { + return NULL; + } else if (client.get() != NULL) { + return client; + } - int facing = -1; - int deviceVersion = getDeviceVersion(cameraId, &facing); + int facing = -1; + int deviceVersion = getDeviceVersion(cameraId, &facing); - // If there are other non-exclusive users of the camera, - // this will tear them down before we can reuse the camera - if (isValidCameraId(cameraId)) { - updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE, cameraId); - } + // If there are other non-exclusive users of the camera, + // this will tear them down before we can reuse the camera + if (isValidCameraId(cameraId)) { + updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE, + cameraId); + } - switch(deviceVersion) { - case CAMERA_DEVICE_API_VERSION_1_0: - client = new CameraClient(this, cameraClient, - clientPackageName, cameraId, - facing, callingPid, clientUid, getpid()); - break; - case CAMERA_DEVICE_API_VERSION_2_0: - case CAMERA_DEVICE_API_VERSION_2_1: - case CAMERA_DEVICE_API_VERSION_3_0: - client = new Camera2Client(this, cameraClient, - clientPackageName, cameraId, - facing, callingPid, clientUid, getpid(), - deviceVersion); - break; - case -1: - ALOGE("Invalid camera id %d", cameraId); - return NULL; - default: - ALOGE("Unknown camera device HAL version: %d", deviceVersion); - return NULL; - } + switch(deviceVersion) { + case CAMERA_DEVICE_API_VERSION_1_0: + client = new CameraClient(this, cameraClient, + clientPackageName, cameraId, + facing, callingPid, clientUid, getpid()); + break; + case CAMERA_DEVICE_API_VERSION_2_0: + case CAMERA_DEVICE_API_VERSION_2_1: + case CAMERA_DEVICE_API_VERSION_3_0: + client = new Camera2Client(this, cameraClient, + clientPackageName, cameraId, + facing, callingPid, clientUid, getpid(), + deviceVersion); + break; + case -1: + ALOGE("Invalid camera id %d", cameraId); + return NULL; + default: + ALOGE("Unknown camera device HAL version: %d", deviceVersion); + return NULL; + } - if (!connectFinishUnsafe(client, client->asBinder())) { - // this is probably not recoverable.. but maybe the client can try again - updateStatus(ICameraServiceListener::STATUS_AVAILABLE, cameraId); + if (!connectFinishUnsafe(client, client->asBinder())) { + // this is probably not recoverable.. maybe the client can try again + updateStatus(ICameraServiceListener::STATUS_AVAILABLE, cameraId); - return NULL; - } + return NULL; + } - mClient[cameraId] = client; - LOG1("CameraService::connect X (id %d, this pid is %d)", cameraId, getpid()); + mClient[cameraId] = client; + LOG1("CameraService::connect X (id %d, this pid is %d)", cameraId, + getpid()); + } + // important: release the mutex here so the client can call back + // into the service from its destructor (can be at the end of the call) return client; } @@ -357,47 +363,51 @@ sp<IProCameraUser> CameraService::connect( return NULL; } - Mutex::Autolock lock(mServiceLock); + sp<ProClient> client; { - sp<Client> client; - if (!canConnectUnsafe(cameraId, clientPackageName, - cameraCb->asBinder(), - /*out*/client)) { - return NULL; + Mutex::Autolock lock(mServiceLock); + { + sp<Client> client; + if (!canConnectUnsafe(cameraId, clientPackageName, + cameraCb->asBinder(), + /*out*/client)) { + return NULL; + } } - } - sp<ProClient> client; + int facing = -1; + int deviceVersion = getDeviceVersion(cameraId, &facing); - int facing = -1; - int deviceVersion = getDeviceVersion(cameraId, &facing); - - switch(deviceVersion) { - case CAMERA_DEVICE_API_VERSION_1_0: - ALOGE("Camera id %d uses HALv1, doesn't support ProCamera", cameraId); - return NULL; - break; - case CAMERA_DEVICE_API_VERSION_2_0: - case CAMERA_DEVICE_API_VERSION_2_1: - client = new ProCamera2Client(this, cameraCb, String16(), - cameraId, facing, callingPid, USE_CALLING_UID, getpid()); - break; - case -1: - ALOGE("Invalid camera id %d", cameraId); - return NULL; - default: - ALOGE("Unknown camera device HAL version: %d", deviceVersion); - return NULL; - } + switch(deviceVersion) { + case CAMERA_DEVICE_API_VERSION_1_0: + ALOGE("Camera id %d uses HALv1, doesn't support ProCamera", + cameraId); + return NULL; + break; + case CAMERA_DEVICE_API_VERSION_2_0: + case CAMERA_DEVICE_API_VERSION_2_1: + client = new ProCamera2Client(this, cameraCb, String16(), + cameraId, facing, callingPid, USE_CALLING_UID, getpid()); + break; + case -1: + ALOGE("Invalid camera id %d", cameraId); + return NULL; + default: + ALOGE("Unknown camera device HAL version: %d", deviceVersion); + return NULL; + } - if (!connectFinishUnsafe(client, client->asBinder())) { - return NULL; - } + if (!connectFinishUnsafe(client, client->asBinder())) { + return NULL; + } - mProClientList[cameraId].push(client); + mProClientList[cameraId].push(client); - LOG1("CameraService::connectPro X (id %d, this pid is %d)", cameraId, - getpid()); + LOG1("CameraService::connectPro X (id %d, this pid is %d)", cameraId, + getpid()); + } + // important: release the mutex here so the client can call back + // into the service from its destructor (can be at the end of the call) return client; } |