commit f1c2d3f3f90a96c313ff6fbdd8e0a612811c679b
author Wan-Teh Chang <wtc@google.com> Mon Jan 09 16:30:15 2023 -0800
committer Wan-Teh Chang <wtc@google.com> Tue Jan 10 17:34:13 2023 +0000
tree a29aecedbed62f0279ed4fea93ce0281d81f33e7
parent c560ae31b3b9e29b5c3e854d9aae1a77d45fe7bf

Do not return a partially-initialized BufferPool

Do not return a partially-initialized BufferPool struct, because we
cannot tell if the pool_mutex member of the struct has been initialized
by a pthread_mutex_init() call. Although the pool_mutex member is
zero-initialized by aom_calloc(), it may not be considered as
initialized by the pthread library, so if pthread_mutex_init() is not
called to initialize pool_mutex, the pthread_mutex_destroy() call on
pool_mutex may fail.

An alternative solution is to add a `bool pool_mutex_initialized` member
to the BufferPool struct.

Change-Id: I41d26a992956c84562196ac12585f6591b372f37

av1/av1_cx_iface.c

diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c
index 470787a..a1f66ca 100644
--- a/av1/av1_cx_iface.c
+++ b/av1/av1_cx_iface.c
@@ -2524,20 +2524,25 @@
   if (buffer_pool == NULL) {
     buffer_pool = (BufferPool *)aom_calloc(1, sizeof(BufferPool));
     if (buffer_pool == NULL) return AOM_CODEC_MEM_ERROR;
-    *p_buffer_pool = buffer_pool;
     buffer_pool->num_frame_bufs =
         (oxcf->mode == ALLINTRA) ? FRAME_BUFFERS_ALLINTRA : FRAME_BUFFERS;
     buffer_pool->frame_bufs = (RefCntBuffer *)aom_calloc(
         buffer_pool->num_frame_bufs, sizeof(*buffer_pool->frame_bufs));
     if (buffer_pool->frame_bufs == NULL) {
       buffer_pool->num_frame_bufs = 0;
+      aom_free(buffer_pool);
       return AOM_CODEC_MEM_ERROR;
     }
 #if CONFIG_MULTITHREAD
     if (pthread_mutex_init(&buffer_pool->pool_mutex, NULL)) {
+      aom_free(buffer_pool->frame_bufs);
+      buffer_pool->frame_bufs = NULL;
+      buffer_pool->num_frame_bufs = 0;
+      aom_free(buffer_pool);
       return AOM_CODEC_MEM_ERROR;
     }
 #endif
+    *p_buffer_pool = buffer_pool;
   }
   *p_cpi =
       av1_create_compressor(ppi, oxcf, buffer_pool, stage, lap_lag_in_frames);

av1/av1_dx_iface.c

diff --git a/av1/av1_dx_iface.c b/av1/av1_dx_iface.c
index 79ce988..1c9b5d2 100644
--- a/av1/av1_dx_iface.c
+++ b/av1/av1_dx_iface.c
@@ -128,9 +128,6 @@
       av1_decoder_remove(pbi);
     }
     aom_free(frame_worker_data);
-#if CONFIG_MULTITHREAD
-    pthread_mutex_destroy(&ctx->buffer_pool->pool_mutex);
-#endif
   }
 
   if (ctx->buffer_pool) {
@@ -140,6 +137,9 @@
     }
     av1_free_ref_frame_buffers(ctx->buffer_pool);
     av1_free_internal_frame_buffers(&ctx->buffer_pool->int_frame_buffers);
+#if CONFIG_MULTITHREAD
+    pthread_mutex_destroy(&ctx->buffer_pool->pool_mutex);
+#endif
   }
 
   aom_free(ctx->frame_worker);
@@ -433,11 +433,18 @@
       ctx->buffer_pool->num_frame_bufs, sizeof(*ctx->buffer_pool->frame_bufs));
   if (ctx->buffer_pool->frame_bufs == NULL) {
     ctx->buffer_pool->num_frame_bufs = 0;
+    aom_free(ctx->buffer_pool);
+    ctx->buffer_pool = NULL;
     return AOM_CODEC_MEM_ERROR;
   }
 
 #if CONFIG_MULTITHREAD
   if (pthread_mutex_init(&ctx->buffer_pool->pool_mutex, NULL)) {
+    aom_free(ctx->buffer_pool->frame_bufs);
+    ctx->buffer_pool->frame_bufs = NULL;
+    ctx->buffer_pool->num_frame_bufs = 0;
+    aom_free(ctx->buffer_pool);
+    ctx->buffer_pool = NULL;
     set_error_detail(ctx, "Failed to allocate buffer pool mutex");
     return AOM_CODEC_MEM_ERROR;
   }