Don't callback on NULL client. Bug 2180510.

We weren't checking to see if there was a valid camera client when
calling the notify callback function. Now we grab a strong pointer
before the callback to guarantee that the client is not destroyed
before we complete the callback. This change also fixes other
places in the code where we weren't holding a local strong pointer.

2 files changed, 63 insertions, 30 deletions

diff --git a/camera/libcameraservice/CameraService.cpp b/camera/libcameraservice/CameraService.cpp
index b63e97f..6880144 100644
--- a/camera/libcameraservice/CameraService.cpp
+++ b/camera/libcameraservice/CameraService.cpp
@@ -309,7 +309,7 @@ status_t CameraService::Client::connect(const sp<ICameraClient>& client)
             oldClient = mCameraClient;
 
             // did the client actually change?
-            if (client->asBinder() == mCameraClient->asBinder()) {
+            if ((mCameraClient != NULL) && (client->asBinder() == mCameraClient->asBinder())) {
                 LOGD("Connect to the same client");
                 return NO_ERROR;
             }
@@ -878,7 +878,10 @@ void CameraService::Client::handleShutter()
         mSurface->unregisterBuffers();
     }
 
-    mCameraClient->notifyCallback(CAMERA_MSG_SHUTTER, 0, 0);
+    sp<ICameraClient> c = mCameraClient;
+    if (c != NULL) {
+        c->notifyCallback(CAMERA_MSG_SHUTTER, 0, 0);
+    }
     mHardware->disableMsgType(CAMERA_MSG_SHUTTER);
 
     // It takes some time before yuvPicture callback to be called.
@@ -932,31 +935,38 @@ void CameraService::Client::handlePreviewData(const sp<IMemory>& mem)
         }
     }
 
-    // Is the callback enabled or not?
-    if (!(mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ENABLE_MASK)) {
+    // local copy of the callback flags
+    int flags = mPreviewCallbackFlag;
+
+    // is callback enabled?
+    if (!(flags & FRAME_CALLBACK_FLAG_ENABLE_MASK)) {
         // If the enable bit is off, the copy-out and one-shot bits are ignored
         LOGV("frame callback is diabled");
         return;
     }
 
-    // Is the received frame copied out or not?
-    if (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_COPY_OUT_MASK) {
-        LOGV("frame is copied out");
-        copyFrameAndPostCopiedFrame(heap, offset, size);
-    } else {
-        LOGV("frame is directly sent out without copying");
-        mCameraClient->dataCallback(CAMERA_MSG_PREVIEW_FRAME, mem);
-    }
+    // hold a strong pointer to the client
+    sp<ICameraClient> c = mCameraClient;
 
-    // Is this is one-shot only?
-    if (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ONE_SHOT_MASK) {
-        LOGV("One-shot only, thus clear the bits and disable frame callback");
+    // clear callback flags if no client or one-shot mode
+    if ((c == NULL) || (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ONE_SHOT_MASK)) {
+        LOGV("Disable preview callback");
         mPreviewCallbackFlag &= ~(FRAME_CALLBACK_FLAG_ONE_SHOT_MASK |
                                 FRAME_CALLBACK_FLAG_COPY_OUT_MASK |
                                 FRAME_CALLBACK_FLAG_ENABLE_MASK);
+        // TODO: Shouldn't we use this API for non-overlay hardware as well?
         if (mUseOverlay)
             mHardware->disableMsgType(CAMERA_MSG_PREVIEW_FRAME);
     }
+
+    // Is the received frame copied out or not?
+    if (flags & FRAME_CALLBACK_FLAG_COPY_OUT_MASK) {
+        LOGV("frame is copied");
+        copyFrameAndPostCopiedFrame(c, heap, offset, size);
+    } else {
+        LOGV("frame is forwarded");
+        c->dataCallback(CAMERA_MSG_PREVIEW_FRAME, mem);
+    }
 }
 
 // picture callback - postview image ready
@@ -972,7 +982,10 @@ void CameraService::Client::handlePostview(const sp<IMemory>& mem)
     }
 #endif
 
-    mCameraClient->dataCallback(CAMERA_MSG_POSTVIEW_FRAME, mem);
+    sp<ICameraClient> c = mCameraClient;
+    if (c != NULL) {
+        c->dataCallback(CAMERA_MSG_POSTVIEW_FRAME, mem);
+    }
     mHardware->disableMsgType(CAMERA_MSG_POSTVIEW_FRAME);
 }
 
@@ -997,7 +1010,10 @@ void CameraService::Client::handleRawPicture(const sp<IMemory>& mem)
         mSurface->postBuffer(offset);
     }
 
-    mCameraClient->dataCallback(CAMERA_MSG_RAW_IMAGE, mem);
+    sp<ICameraClient> c = mCameraClient;
+    if (c != NULL) {
+        c->dataCallback(CAMERA_MSG_RAW_IMAGE, mem);
+    }
     mHardware->disableMsgType(CAMERA_MSG_RAW_IMAGE);
 }
 
@@ -1014,7 +1030,10 @@ void CameraService::Client::handleCompressedPicture(const sp<IMemory>& mem)
     }
 #endif
 
-    mCameraClient->dataCallback(CAMERA_MSG_COMPRESSED_IMAGE, mem);
+    sp<ICameraClient> c = mCameraClient;
+    if (c != NULL) {
+        c->dataCallback(CAMERA_MSG_COMPRESSED_IMAGE, mem);
+    }
     mHardware->disableMsgType(CAMERA_MSG_COMPRESSED_IMAGE);
 }
 
@@ -1032,7 +1051,10 @@ void CameraService::Client::notifyCallback(int32_t msgType, int32_t ext1, int32_
             client->handleShutter();
             break;
         default:
-            client->mCameraClient->notifyCallback(msgType, ext1, ext2);
+            sp<ICameraClient> c = client->mCameraClient;
+            if (c != NULL) {
+                c->notifyCallback(msgType, ext1, ext2);
+            }
             break;
     }
 
@@ -1053,10 +1075,13 @@ void CameraService::Client::dataCallback(int32_t msgType, const sp<IMemory>& dat
         return;
     }
 
+    sp<ICameraClient> c = client->mCameraClient;
     if (dataPtr == NULL) {
         LOGE("Null data returned in data callback");
-        client->mCameraClient->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);
-        client->mCameraClient->dataCallback(msgType, NULL);
+        if (c != NULL) {
+            c->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);
+            c->dataCallback(msgType, NULL);
+        }
         return;
     }
 
@@ -1074,7 +1099,9 @@ void CameraService::Client::dataCallback(int32_t msgType, const sp<IMemory>& dat
             client->handleCompressedPicture(dataPtr);
             break;
         default:
-            client->mCameraClient->dataCallback(msgType, dataPtr);
+            if (c != NULL) {
+                c->dataCallback(msgType, dataPtr);
+            }
             break;
     }
 
@@ -1095,15 +1122,20 @@ void CameraService::Client::dataCallbackTimestamp(nsecs_t timestamp, int32_t msg
     if (client == 0) {
         return;
     }
+    sp<ICameraClient> c = client->mCameraClient;
 
     if (dataPtr == NULL) {
         LOGE("Null data returned in data with timestamp callback");
-        client->mCameraClient->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);
-        client->mCameraClient->dataCallbackTimestamp(0, msgType, NULL);
+        if (c != NULL) {
+            c->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);
+            c->dataCallbackTimestamp(0, msgType, NULL);
+        }
         return;
     }
 
-    client->mCameraClient->dataCallbackTimestamp(timestamp, msgType, dataPtr);
+    if (c != NULL) {
+        c->dataCallbackTimestamp(timestamp, msgType, dataPtr);
+    }
 
 #if DEBUG_CLIENT_REFERENCES
     if (client->getStrongCount() == 1) {
@@ -1161,7 +1193,8 @@ status_t CameraService::Client::sendCommand(int32_t cmd, int32_t arg1, int32_t a
     return mHardware->sendCommand(cmd, arg1, arg2);
 }
 
-void CameraService::Client::copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, size_t offset, size_t size)
+void CameraService::Client::copyFrameAndPostCopiedFrame(const sp<ICameraClient>& client,
+        const sp<IMemoryHeap>& heap, size_t offset, size_t size)
 {
     LOGV("copyFrameAndPostCopiedFrame");
     // It is necessary to copy out of pmem before sending this to
@@ -1186,7 +1219,7 @@ void CameraService::Client::copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, si
         LOGE("failed to allocate space for frame callback");
         return;
     }
-    mCameraClient->dataCallback(CAMERA_MSG_PREVIEW_FRAME, frame);
+    client->dataCallback(CAMERA_MSG_PREVIEW_FRAME, frame);
 }
 
 status_t CameraService::dump(int fd, const Vector<String16>& args)
diff --git a/camera/libcameraservice/CameraService.h b/camera/libcameraservice/CameraService.h
index 2e3597f..2fcf839 100644
--- a/camera/libcameraservice/CameraService.h
+++ b/camera/libcameraservice/CameraService.h
@@ -23,10 +23,9 @@
 #include <ui/CameraHardwareInterface.h>
 #include <ui/Camera.h>
 
-class android::MemoryHeapBase;
-
 namespace android {
 
+class MemoryHeapBase;
 class MediaPlayer;
 
 // ----------------------------------------------------------------------------
@@ -151,7 +150,8 @@ private:
                     void        handleRawPicture(const sp<IMemory>&);
                     void        handleCompressedPicture(const sp<IMemory>&);
 
-                    void        copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, size_t offset, size_t size);
+                    void        copyFrameAndPostCopiedFrame(const sp<ICameraClient>& client,
+                                    const sp<IMemoryHeap>& heap, size_t offset, size_t size);
 
         // camera operation mode
         enum camera_mode {