Adopt weak pointers for death detection in EVS
Keep only weak pointers on the server side to objects that are owned by
the client. Use the weak pointers to detect if/when the client goes away
unexpectedly. Use destructors to do server side cleanup when the last
client reference dies.
Also removes explicit handle cloning as that is now handled inside
hidl_handle (as of 1/25/2017)
Adds calls to register buffer to comply with gralloc requirements.
Test: locally compile and run evs_test.
Change-Id: I18fc0df3fa643536ab5dbc89d52de9af6a286972
diff --git a/evs/manager/Enumerator.cpp b/evs/manager/Enumerator.cpp
index c710c92..b7f6446 100644
--- a/evs/manager/Enumerator.cpp
+++ b/evs/manager/Enumerator.cpp
@@ -85,14 +85,14 @@
clientCamera = hwCamera->makeVirtualCamera();
}
- // Add the hardware camera to our list, which will keep the object alive via ref count
+ // Add the hardware camera to our list, which will keep it alive via ref count
if (clientCamera != nullptr) {
mCameras.push_back(hwCamera);
} else {
ALOGE("Requested camera %s not found or not available", cameraId.c_str());
}
- // Send the virtual camera object (also owned by our hwCamera) back to the client
+ // Send the virtual camera object back to the client by strong pointer which will keep it alive
return clientCamera;
}
@@ -136,19 +136,20 @@
ALOGD("openDisplay");
// If we already have a display active, then this request must be denied
- if (mActiveDisplay != nullptr) {
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+ if (pActiveDisplay != nullptr) {
ALOGW("Rejecting openDisplay request because the display is already in use.");
return nullptr;
} else {
// Request exclusive access to the EVS display
ALOGI("Acquiring EVS Display");
-
- mActiveDisplay = mHwEnumerator->openDisplay();
- if (mActiveDisplay.get() == nullptr) {
+ pActiveDisplay = mHwEnumerator->openDisplay();
+ if (pActiveDisplay == nullptr) {
ALOGE("EVS Display unavailable");
}
- return mActiveDisplay;
+ mActiveDisplay = pActiveDisplay;
+ return pActiveDisplay;
}
}
@@ -156,9 +157,13 @@
Return<void> Enumerator::closeDisplay(const ::android::sp<IEvsDisplay>& display) {
ALOGD("closeDisplay");
+ // Do we still have a display object we think should be active?
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+
// Drop the active display
- if (display != mActiveDisplay) {
- ALOGW("Ignoring call to closeDisplay with a display object we don't recognize.");
+ if (display.get() != pActiveDisplay.get()) {
+ ALOGW("Ignoring call to closeDisplay with unrecognzied display object.");
+ ALOGI("Got %p while active display is %p.", display.get(), pActiveDisplay.get());
} else {
// Pass this request through to the hardware layer
mHwEnumerator->closeDisplay(display);
@@ -172,24 +177,20 @@
Return<DisplayState> Enumerator::getDisplayState() {
ALOGD("getDisplayState");
- // Pass this request through to the hardware layer
- if (mHwEnumerator == nullptr) {
- // If we haven't even been initialized, this should not be called
- ALOGE("getDisplayState called on incompletely initialized EvsEnumerator");
- return DisplayState::NOT_OPEN;
- } else if (mActiveDisplay == nullptr) {
- // We actually DO hold the underlying hardware display open (in order to reserve it)
- // but the application hasn't opened it, so we know it isn't actually doing anything.
- return DisplayState::NOT_OPEN;
+ // Do we have a display object we think should be active?
+ sp<IEvsDisplay> pActiveDisplay = mActiveDisplay.promote();
+ if (pActiveDisplay != nullptr) {
+ // Pass this request through to the hardware layer
+ return pActiveDisplay->getDisplayState();
} else {
- return mActiveDisplay->getDisplayState();
+ // We don't have a live display right now
+ mActiveDisplay = nullptr;
+ return DisplayState::NOT_OPEN;
}
}
// TODO(b/31632518): Need to get notification when our client dies so we can close the camera.
-// As possible work around would be to give the client a HIDL object to exclusively hold
-// and use it's destructor to perform some work in the server side.
} // namespace implementation