OSDN Git Service

+rcDestroySyncKHR, don't leak on swapBuffers
authorLingfeng Yang <lfy@google.com>
Wed, 31 Aug 2016 21:46:11 +0000 (14:46 -0700)
committerLingfeng Yang <lfy@google.com>
Sat, 3 Sep 2016 20:12:41 +0000 (13:12 -0700)
This CL is part of this host CL to clean up EGL sync objects:

https://android-review.googlesource.com/#/c/267892/

It does the following:
- adds rcDestroySyncKHR
- avoids creating an actual EGL sync object when in swapBuffers(),
  because it will be cumbersome to delete it from the guest.
  Instead, it specifically calls rcCreateSyncKHR and tells the host
  to clean up the object when it has become signaled.
- refactors rcCreateSyncKHR / goldfish_sync_queue_work to
  a common function.
- bumps the GLAsyncSwap version string to "ANDROID_EMU_native_sync_v2",
  so that the emulator can work with all combinations of
  old/new emulator/system-image.

Change-Id: Iff9a5e226b4026b955893c6f4d6ff3266009da6b

system/OpenglSystemCommon/HostConnection.h
system/egl/egl.cpp
system/renderControl_enc/renderControl_client_context.cpp
system/renderControl_enc/renderControl_client_context.h
system/renderControl_enc/renderControl_client_proc.h
system/renderControl_enc/renderControl_enc.cpp
system/renderControl_enc/renderControl_entry.cpp
system/renderControl_enc/renderControl_ftable.h
system/renderControl_enc/renderControl_opcodes.h

index e01ec28..c058e75 100644 (file)
@@ -44,7 +44,10 @@ enum SyncImpl {
 // Interface:
 // If this GL extension string shows up, we use
 // SYNC_IMPL_NATIVE_SYNC, otherwise we use SYNC_IMPL_NONE.
-static const char kRCNativeSync[] = "ANDROID_EMU_NATIVE_SYNC";
+// This string is always updated to require the _latest_
+// version of Android emulator native sync in this system image;
+// otherwise, we do not use the feature.
+static const char kRCNativeSync[] = "ANDROID_EMU_native_sync_v2";
 
 // ExtendedRCEncoderContext is an extended version of renderControl_encoder_context_t
 // that will be used to track SyncImpl.
index 94dd1a6..39ed259 100644 (file)
@@ -355,6 +355,62 @@ void egl_window_surface_t::setSwapInterval(int interval)
     nativeWindow->setSwapInterval(nativeWindow, interval);
 }
 
+// createNativeSync() creates an OpenGL sync object on the host
+// using rcCreateSyncKHR. If necessary, a native fence FD will
+// also be created through the goldfish sync device.
+// Returns a handle to the host-side FenceSync object.
+static uint64_t createNativeSync(EGLenum type,
+                                 const EGLint* attrib_list,
+                                 int num_actual_attribs,
+                                 bool destroy_when_signaled,
+                                 int fd_in,
+                                 int* fd_out) {
+    DEFINE_HOST_CONNECTION;
+
+    uint64_t sync_handle;
+    uint64_t thread_handle;
+
+    EGLint* actual_attribs =
+        (EGLint*)(num_actual_attribs == 0 ? NULL : attrib_list);
+
+    rcEnc->rcCreateSyncKHR(rcEnc, type,
+                           actual_attribs,
+                           num_actual_attribs * sizeof(EGLint),
+                           destroy_when_signaled,
+                           &sync_handle,
+                           &thread_handle);
+
+    if (type == EGL_SYNC_NATIVE_FENCE_ANDROID && fd_in < 0) {
+        int queue_work_err =
+            goldfish_sync_queue_work(
+                    getEGLThreadInfo()->currentContext->getGoldfishSyncFd(),
+                    sync_handle,
+                    thread_handle,
+                    fd_out);
+
+        DPRINT("got native fence fd=%d queue_work_err=%d",
+               *fd_out, queue_work_err);
+    }
+
+    return sync_handle;
+}
+
+// createGoldfishOpenGLNativeSync() is for creating host-only sync objects
+// that are needed by only this goldfish opengl driver,
+// such as in swapBuffers().
+// The guest will not see any of these, and these sync objects will be
+// destroyed on the host when signaled.
+// A native fence FD is possibly returned.
+static void createGoldfishOpenGLNativeSync(int* fd_out) {
+    createNativeSync(EGL_SYNC_NATIVE_FENCE_ANDROID,
+                     NULL /* empty attrib list */,
+                     0 /* 0 attrib count */,
+                     true /* destroy when signaled. this is host-only
+                             and there will only be one waiter */,
+                     -1 /* we want a new fd */,
+                     fd_out);
+}
+
 EGLBoolean egl_window_surface_t::swapBuffers()
 {
 
@@ -386,11 +442,7 @@ EGLBoolean egl_window_surface_t::swapBuffers()
 #else
     if (rcEnc->hasNativeSync()) {
         rcEnc->rcFlushWindowColorBufferAsync(rcEnc, rcSurface);
-        EGLSyncKHR sync = eglCreateSyncKHR(dpy,
-                                           EGL_SYNC_NATIVE_FENCE_ANDROID,
-                                           NULL);
-        EGLSync_t* syncObj = (EGLSync_t*)sync;
-        presentFenceFd = syncObj->android_native_fence_fd;
+        createGoldfishOpenGLNativeSync(&presentFenceFd);
     } else {
         rcEnc->rcFlushWindowColorBuffer(rcEnc, rcSurface);
         // equivalent to glFinish if no native sync
@@ -1436,7 +1488,6 @@ EGLBoolean eglDestroyImageKHR(EGLDisplay dpy, EGLImageKHR img)
 #define FENCE_SYNC_HANDLE (EGLSyncKHR)0xFE4CE
 #define MAX_EGL_SYNC_ATTRIBS 10
 
-
 EGLSyncKHR eglCreateSyncKHR(EGLDisplay dpy, EGLenum type,
         const EGLint *attrib_list)
 {
@@ -1499,33 +1550,16 @@ EGLSyncKHR eglCreateSyncKHR(EGLDisplay dpy, EGLenum type,
     }
 
     uint64_t sync_handle = 0;
-    int native_fence_fd = -1;
+    int newFenceFd = -1;
 
-    EGLContext_t* cxt = getEGLThreadInfo()->currentContext;
     if (rcEnc->hasNativeSync()) {
-        uint64_t thread_out = 0;
-
-        rcEnc->rcCreateSyncKHR(rcEnc,
-                type,
-                (EGLint*)(num_actual_attribs == 0 ? NULL : attrib_list),
-                num_actual_attribs * sizeof(EGLint),
-                &sync_handle,
-                &thread_out);
-
-        if (sync_handle && thread_out &&
-            (type == EGL_SYNC_NATIVE_FENCE_ANDROID) &&
-            (inputFenceFd < 0)) {
-
-            int goldfish_sync_fd = cxt->getGoldfishSyncFd();
-            int queue_work_err =
-                goldfish_sync_queue_work(goldfish_sync_fd,
-                        sync_handle,
-                        thread_out,
-                        &native_fence_fd);
-
-            DPRINT("got native fence fd=%d queue_work_err=%d",
-                   native_fence_fd, queue_work_err);
-        }
+        sync_handle =
+            createNativeSync(type, attrib_list, num_actual_attribs,
+                             false /* don't destroy when signaled on the host;
+                                      let the guest clean this up,
+                                      because the guest called eglCreateSyncKHR. */,
+                             inputFenceFd,
+                             &newFenceFd);
 
     } else {
         // Just trigger a glFinish if the native sync on host
@@ -1539,7 +1573,7 @@ EGLSyncKHR eglCreateSyncKHR(EGLDisplay dpy, EGLenum type,
         syncRes->type = EGL_SYNC_NATIVE_FENCE_ANDROID;
 
         if (inputFenceFd < 0) {
-            syncRes->android_native_fence_fd = native_fence_fd;
+            syncRes->android_native_fence_fd = newFenceFd;
         } else {
             DPRINT("has input fence fd %d",
                     inputFenceFd);
@@ -1569,7 +1603,13 @@ EGLBoolean eglDestroySyncKHR(EGLDisplay dpy, EGLSyncKHR eglsync)
         sync->android_native_fence_fd = -1;
     }
 
-    if (sync) delete sync;
+    if (sync) {
+        DEFINE_HOST_CONNECTION;
+        if (rcEnc->hasNativeSync()) {
+            rcEnc->rcDestroySyncKHR(rcEnc, sync->handle);
+        }
+        delete sync;
+    }
 
     return EGL_TRUE;
 }
index 8fb6588..a41a7e9 100644 (file)
@@ -47,6 +47,7 @@ int renderControl_client_context_t::initDispatchByName(void *(*getProc)(const ch
        rcFlushWindowColorBufferAsync = (rcFlushWindowColorBufferAsync_client_proc_t) getProc("rcFlushWindowColorBufferAsync", userData);
        rcCreateClientImagePuid = (rcCreateClientImagePuid_client_proc_t) getProc("rcCreateClientImagePuid", userData);
        rcDestroyClientImagePuid = (rcDestroyClientImagePuid_client_proc_t) getProc("rcDestroyClientImagePuid", userData);
+       rcDestroySyncKHR = (rcDestroySyncKHR_client_proc_t) getProc("rcDestroySyncKHR", userData);
        return 0;
 }
 
index 1adf609..df234ce 100644 (file)
@@ -47,6 +47,7 @@ struct renderControl_client_context_t {
        rcFlushWindowColorBufferAsync_client_proc_t rcFlushWindowColorBufferAsync;
        rcCreateClientImagePuid_client_proc_t rcCreateClientImagePuid;
        rcDestroyClientImagePuid_client_proc_t rcDestroyClientImagePuid;
+       rcDestroySyncKHR_client_proc_t rcDestroySyncKHR;
        virtual ~renderControl_client_context_t() {}
 
        typedef renderControl_client_context_t *CONTEXT_ACCESSOR_TYPE(void);
index 7385c3f..47d6755 100644 (file)
@@ -41,11 +41,12 @@ typedef void (renderControl_APIENTRY *rcSelectChecksumHelper_client_proc_t) (voi
 typedef uint32_t (renderControl_APIENTRY *rcCreateColorBufferPuid_client_proc_t) (void * ctx, uint32_t, uint32_t, GLenum, uint64_t);
 typedef int (renderControl_APIENTRY *rcOpenColorBuffer2Puid_client_proc_t) (void * ctx, uint32_t, uint64_t);
 typedef void (renderControl_APIENTRY *rcCloseColorBufferPuid_client_proc_t) (void * ctx, uint32_t, uint64_t);
-typedef void (renderControl_APIENTRY *rcCreateSyncKHR_client_proc_t) (void * ctx, EGLenum, EGLint*, uint32_t, uint64_t*, uint64_t*);
+typedef void (renderControl_APIENTRY *rcCreateSyncKHR_client_proc_t) (void * ctx, EGLenum, EGLint*, uint32_t, int, uint64_t*, uint64_t*);
 typedef EGLint (renderControl_APIENTRY *rcClientWaitSyncKHR_client_proc_t) (void * ctx, uint64_t, EGLint, uint64_t);
 typedef void (renderControl_APIENTRY *rcFlushWindowColorBufferAsync_client_proc_t) (void * ctx, uint32_t);
 typedef uint32_t (renderControl_APIENTRY *rcCreateClientImagePuid_client_proc_t) (void * ctx, uint32_t, EGLenum, GLuint, uint64_t);
 typedef int (renderControl_APIENTRY *rcDestroyClientImagePuid_client_proc_t) (void * ctx, uint32_t, uint64_t);
+typedef int (renderControl_APIENTRY *rcDestroySyncKHR_client_proc_t) (void * ctx, uint64_t);
 
 
 #endif
index d6a2c2e..8899c11 100644 (file)
@@ -1205,7 +1205,7 @@ void rcCloseColorBufferPuid_enc(void *self , uint32_t colorbuffer, uint64_t puid
        stream->flush();
 }
 
-void rcCreateSyncKHR_enc(void *self , EGLenum type, EGLint* attribs, uint32_t num_attribs, uint64_t* glsync_out, uint64_t* syncthread_out)
+void rcCreateSyncKHR_enc(void *self , EGLenum type, EGLint* attribs, uint32_t num_attribs, int destroy_when_signaled, uint64_t* glsync_out, uint64_t* syncthread_out)
 {
 
        renderControl_encoder_context_t *ctx = (renderControl_encoder_context_t *)self;
@@ -1218,7 +1218,7 @@ void rcCreateSyncKHR_enc(void *self , EGLenum type, EGLint* attribs, uint32_t nu
        const unsigned int __size_syncthread_out =  sizeof(uint64_t);
         unsigned char *ptr;
         unsigned char *buf;
-        const size_t sizeWithoutChecksum = 8 + 4 + __size_attribs + 4 + __size_glsync_out + __size_syncthread_out + 3*4;
+        const size_t sizeWithoutChecksum = 8 + 4 + __size_attribs + 4 + 4 + __size_glsync_out + __size_syncthread_out + 3*4;
         const size_t checksumSize = checksumCalculator->checksumByteSize();
         const size_t totalSize = sizeWithoutChecksum + checksumSize;
        buf = stream->alloc(totalSize);
@@ -1230,6 +1230,7 @@ void rcCreateSyncKHR_enc(void *self , EGLenum type, EGLint* attribs, uint32_t nu
        *(unsigned int *)(ptr) = __size_attribs; ptr += 4;
        memcpy(ptr, attribs, __size_attribs);ptr += __size_attribs;
                memcpy(ptr, &num_attribs, 4); ptr += 4;
+               memcpy(ptr, &destroy_when_signaled, 4); ptr += 4;
        *(unsigned int *)(ptr) = __size_glsync_out; ptr += 4;
        *(unsigned int *)(ptr) = __size_syncthread_out; ptr += 4;
 
@@ -1403,6 +1404,46 @@ int rcDestroyClientImagePuid_enc(void *self , uint32_t image, uint64_t puid)
        return retval;
 }
 
+int rcDestroySyncKHR_enc(void *self , uint64_t sync)
+{
+
+       renderControl_encoder_context_t *ctx = (renderControl_encoder_context_t *)self;
+       IOStream *stream = ctx->m_stream;
+       ChecksumCalculator *checksumCalculator = ctx->m_checksumCalculator;
+       bool useChecksum = checksumCalculator->getVersion() > 0;
+
+        unsigned char *ptr;
+        unsigned char *buf;
+        const size_t sizeWithoutChecksum = 8 + 8;
+        const size_t checksumSize = checksumCalculator->checksumByteSize();
+        const size_t totalSize = sizeWithoutChecksum + checksumSize;
+       buf = stream->alloc(totalSize);
+       ptr = buf;
+       int tmp = OP_rcDestroySyncKHR;memcpy(ptr, &tmp, 4); ptr += 4;
+       memcpy(ptr, &totalSize, 4);  ptr += 4;
+
+               memcpy(ptr, &sync, 8); ptr += 8;
+
+       if (useChecksum) checksumCalculator->addBuffer(buf, ptr-buf);
+       if (useChecksum) checksumCalculator->writeChecksum(ptr, checksumSize); ptr += checksumSize;
+
+
+       int retval;
+       stream->readback(&retval, 4);
+       if (useChecksum) checksumCalculator->addBuffer(&retval, 4);
+       if (useChecksum) {
+               unsigned char *checksumBufPtr = NULL;
+               std::vector<unsigned char> checksumBuf(checksumSize);
+               if (checksumSize > 0) checksumBufPtr = &checksumBuf[0];
+               stream->readback(checksumBufPtr, checksumSize);
+               if (!checksumCalculator->validate(checksumBufPtr, checksumSize)) {
+                       ALOGE("rcDestroySyncKHR: GL communication error, please report this issue to b.android.com.\n");
+                       abort();
+               }
+       }
+       return retval;
+}
+
 }  // namespace
 
 renderControl_encoder_context_t::renderControl_encoder_context_t(IOStream *stream, ChecksumCalculator *checksumCalculator)
@@ -1447,5 +1488,6 @@ renderControl_encoder_context_t::renderControl_encoder_context_t(IOStream *strea
        this->rcFlushWindowColorBufferAsync = &rcFlushWindowColorBufferAsync_enc;
        this->rcCreateClientImagePuid = &rcCreateClientImagePuid_enc;
        this->rcDestroyClientImagePuid = &rcDestroyClientImagePuid_enc;
+       this->rcDestroySyncKHR = &rcDestroySyncKHR_enc;
 }
 
index b2887c5..948703b 100644 (file)
@@ -38,11 +38,12 @@ extern "C" {
        uint32_t rcCreateColorBufferPuid(uint32_t width, uint32_t height, GLenum internalFormat, uint64_t puid);
        int rcOpenColorBuffer2Puid(uint32_t colorbuffer, uint64_t puid);
        void rcCloseColorBufferPuid(uint32_t colorbuffer, uint64_t puid);
-       void rcCreateSyncKHR(EGLenum type, EGLint* attribs, uint32_t num_attribs, uint64_t* glsync_out, uint64_t* syncthread_out);
+       void rcCreateSyncKHR(EGLenum type, EGLint* attribs, uint32_t num_attribs, int destroy_when_signaled, uint64_t* glsync_out, uint64_t* syncthread_out);
        EGLint rcClientWaitSyncKHR(uint64_t sync, EGLint flags, uint64_t timeout);
        void rcFlushWindowColorBufferAsync(uint32_t windowSurface);
        uint32_t rcCreateClientImagePuid(uint32_t context, EGLenum target, GLuint buffer, uint64_t puid);
        int rcDestroyClientImagePuid(uint32_t image, uint64_t puid);
+       int rcDestroySyncKHR(uint64_t sync);
 };
 
 #endif
@@ -244,10 +245,10 @@ void rcCloseColorBufferPuid(uint32_t colorbuffer, uint64_t puid)
        ctx->rcCloseColorBufferPuid(ctx, colorbuffer, puid);
 }
 
-void rcCreateSyncKHR(EGLenum type, EGLint* attribs, uint32_t num_attribs, uint64_t* glsync_out, uint64_t* syncthread_out)
+void rcCreateSyncKHR(EGLenum type, EGLint* attribs, uint32_t num_attribs, int destroy_when_signaled, uint64_t* glsync_out, uint64_t* syncthread_out)
 {
        GET_CONTEXT;
-       ctx->rcCreateSyncKHR(ctx, type, attribs, num_attribs, glsync_out, syncthread_out);
+       ctx->rcCreateSyncKHR(ctx, type, attribs, num_attribs, destroy_when_signaled, glsync_out, syncthread_out);
 }
 
 EGLint rcClientWaitSyncKHR(uint64_t sync, EGLint flags, uint64_t timeout)
@@ -274,3 +275,9 @@ int rcDestroyClientImagePuid(uint32_t image, uint64_t puid)
        return ctx->rcDestroyClientImagePuid(ctx, image, puid);
 }
 
+int rcDestroySyncKHR(uint64_t sync)
+{
+       GET_CONTEXT;
+       return ctx->rcDestroySyncKHR(ctx, sync);
+}
+
index 777e99e..ed6e637 100644 (file)
@@ -45,6 +45,7 @@ static const struct _renderControl_funcs_by_name {
        {"rcFlushWindowColorBufferAsync", (void*)rcFlushWindowColorBufferAsync},
        {"rcCreateClientImagePuid", (void*)rcCreateClientImagePuid},
        {"rcDestroyClientImagePuid", (void*)rcDestroyClientImagePuid},
+       {"rcDestroySyncKHR", (void*)rcDestroySyncKHR},
 };
 static const int renderControl_num_funcs = sizeof(renderControl_funcs_by_name) / sizeof(struct _renderControl_funcs_by_name);
 
index 5ce4a75..ad6ca13 100644 (file)
@@ -40,7 +40,8 @@
 #define OP_rcFlushWindowColorBufferAsync                                       10034
 #define OP_rcCreateClientImagePuid                                     10035
 #define OP_rcDestroyClientImagePuid                                    10036
-#define OP_last                                        10037
+#define OP_rcDestroySyncKHR                                    10037
+#define OP_last                                        10038
 
 
 #endif