diff options
author | Ruben Brunk <rubenbrunk@google.com> | 2015-08-13 23:05:08 +0000 |
---|---|---|
committer | Android Git Automerger <android-git-automerger@android.com> | 2015-08-13 23:05:08 +0000 |
commit | b65ce67e4083b2026c9f31cb762abe4be39f1560 (patch) | |
tree | 44d991e522eeb1bd3d1a0bf2d265be807dd8fbb6 /services | |
parent | b9d82b0c5048e73dfe354b337a849e814908d9f7 (diff) | |
parent | 3ba0c6ec009aa675d3433cd6c12af173874787bd (diff) | |
download | frameworks_av-b65ce67e4083b2026c9f31cb762abe4be39f1560.zip frameworks_av-b65ce67e4083b2026c9f31cb762abe4be39f1560.tar.gz frameworks_av-b65ce67e4083b2026c9f31cb762abe4be39f1560.tar.bz2 |
am 3ba0c6ec: Merge "Fix deadlock conditions in Camera3Device." into mnc-dev
* commit '3ba0c6ec009aa675d3433cd6c12af173874787bd':
Fix deadlock conditions in Camera3Device.
Diffstat (limited to 'services')
-rw-r--r-- | services/camera/libcameraservice/device3/Camera3Device.cpp | 63 | ||||
-rw-r--r-- | services/camera/libcameraservice/device3/Camera3Device.h | 11 |
2 files changed, 54 insertions, 20 deletions
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp index 0a4440f..0c941fb 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.cpp +++ b/services/camera/libcameraservice/device3/Camera3Device.cpp @@ -60,6 +60,7 @@ Camera3Device::Camera3Device(int id): mIsConstrainedHighSpeedConfiguration(false), mHal3Device(NULL), mStatus(STATUS_UNINITIALIZED), + mStatusWaiters(0), mUsePartialResult(false), mNumPartialResults(1), mNextResultFrameNumber(0), @@ -191,7 +192,8 @@ status_t Camera3Device::initialize(CameraModule *module) mDeviceVersion = device->common.version; mDeviceInfo = info.static_camera_characteristics; mHal3Device = device; - mStatus = STATUS_UNCONFIGURED; + + internalUpdateStatusLocked(STATUS_UNCONFIGURED); mNextStreamId = 0; mDummyStreamId = NO_STREAM; mNeedConfig = true; @@ -296,7 +298,7 @@ status_t Camera3Device::disconnect() { mHal3Device = NULL; } - mStatus = STATUS_UNINITIALIZED; + internalUpdateStatusLocked(STATUS_UNINITIALIZED); } ALOGV("%s: X", __FUNCTION__); @@ -1166,6 +1168,13 @@ status_t Camera3Device::waitUntilDrainedLocked() { return res; } + +void Camera3Device::internalUpdateStatusLocked(Status status) { + mStatus = status; + mRecentStatusUpdates.add(mStatus); + mStatusChanged.broadcast(); +} + // Pause to reconfigure status_t Camera3Device::internalPauseAndWaitLocked() { mRequestThread->setPaused(true); @@ -1196,23 +1205,40 @@ status_t Camera3Device::internalResumeLocked() { return OK; } -status_t Camera3Device::waitUntilStateThenRelock(bool active, - nsecs_t timeout) { +status_t Camera3Device::waitUntilStateThenRelock(bool active, nsecs_t timeout) { status_t res = OK; - if (active == (mStatus == STATUS_ACTIVE)) { - // Desired state already reached - return res; + + size_t startIndex = 0; + if (mStatusWaiters == 0) { + // Clear the list of recent statuses if there are no existing threads waiting on updates to + // this status list + mRecentStatusUpdates.clear(); + } else { + // If other threads are waiting on updates to this status list, set the position of the + // first element that this list will check rather than clearing the list. + startIndex = mRecentStatusUpdates.size(); } + mStatusWaiters++; + bool stateSeen = false; do { - mRecentStatusUpdates.clear(); + if (active == (mStatus == STATUS_ACTIVE)) { + // Desired state is current + break; + } res = mStatusChanged.waitRelative(mLock, timeout); if (res != OK) break; - // Check state change history during wait - for (size_t i = 0; i < mRecentStatusUpdates.size(); i++) { + // This is impossible, but if not, could result in subtle deadlocks and invalid state + // transitions. + LOG_ALWAYS_FATAL_IF(startIndex > mRecentStatusUpdates.size(), + "%s: Skipping status updates in Camera3Device, may result in deadlock.", + __FUNCTION__); + + // Encountered desired state since we began waiting + for (size_t i = startIndex; i < mRecentStatusUpdates.size(); i++) { if (active == (mRecentStatusUpdates[i] == STATUS_ACTIVE) ) { stateSeen = true; break; @@ -1220,6 +1246,8 @@ status_t Camera3Device::waitUntilStateThenRelock(bool active, } } while (!stateSeen); + mStatusWaiters--; + return res; } @@ -1461,9 +1489,7 @@ void Camera3Device::notifyStatus(bool idle) { } ALOGV("%s: Camera %d: Now %s", __FUNCTION__, mId, idle ? "idle" : "active"); - mStatus = idle ? STATUS_CONFIGURED : STATUS_ACTIVE; - mRecentStatusUpdates.add(mStatus); - mStatusChanged.signal(); + internalUpdateStatusLocked(idle ? STATUS_CONFIGURED : STATUS_ACTIVE); // Skip notifying listener if we're doing some user-transparent // state changes @@ -1672,7 +1698,7 @@ status_t Camera3Device::configureStreamsLocked() { // Return state to that at start of call, so that future configures // properly clean things up - mStatus = STATUS_UNCONFIGURED; + internalUpdateStatusLocked(STATUS_UNCONFIGURED); mNeedConfig = true; ALOGV("%s: Camera %d: Stream configuration failed", __FUNCTION__, mId); @@ -1719,11 +1745,8 @@ status_t Camera3Device::configureStreamsLocked() { mNeedConfig = false; - if (mDummyStreamId == NO_STREAM) { - mStatus = STATUS_CONFIGURED; - } else { - mStatus = STATUS_UNCONFIGURED; - } + internalUpdateStatusLocked((mDummyStreamId == NO_STREAM) ? + STATUS_CONFIGURED : STATUS_UNCONFIGURED); ALOGV("%s: Camera %d: Stream configuration complete", __FUNCTION__, mId); @@ -1831,7 +1854,7 @@ void Camera3Device::setErrorStateLockedV(const char *fmt, va_list args) { mErrorCause = errorCause; mRequestThread->setPaused(true); - mStatus = STATUS_ERROR; + internalUpdateStatusLocked(STATUS_ERROR); // Notify upstream about a device error if (mListener != NULL) { diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h index 8bd0f8e..5287058 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.h +++ b/services/camera/libcameraservice/device3/Camera3Device.h @@ -207,7 +207,11 @@ class Camera3Device : STATUS_CONFIGURED, STATUS_ACTIVE } mStatus; + + // Only clear mRecentStatusUpdates, mStatusWaiters from waitUntilStateThenRelock Vector<Status> mRecentStatusUpdates; + int mStatusWaiters; + Condition mStatusChanged; // Tracking cause of fatal errors when in STATUS_ERROR @@ -278,6 +282,13 @@ class Camera3Device : virtual CameraMetadata getLatestRequestLocked(); /** + * Update the current device status and wake all waiting threads. + * + * Must be called with mLock held. + */ + void internalUpdateStatusLocked(Status status); + + /** * Pause processing and flush everything, but don't tell the clients. * This is for reconfiguring outputs transparently when according to the * CameraDeviceBase interface we shouldn't need to. |