From: Yann Collet <cyan@fb.com>
Date: Wed, 11 Jan 2017 14:44:26 +0000 (+0100)
Subject: improved ZSTD_createCCtxPool() cancellation
X-Git-Tag: v1.1.3^2~19^2~50
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8ce1cc2bec798fb959dcc403b462eebf3a985119;p=thirdparty%2Fzstd.git

improved ZSTD_createCCtxPool() cancellation

use ZSTD_freeCCtxPool() to release the partially created pool.
avoids to duplicate logic.

Also : identified a new difficult corner case :
when freeing the Pool, all CCtx should be previously released back to the pool.
Otherwise, it means some CCtx are still in use.
There is currently no clear policy on what to do in such a case.
Note : it's supposed to never happen.
Since pool creation/usage is static, it has no external user,
which limits risks.
---

diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c
index 9657bdc62..5c7b654ab 100644
--- a/lib/compress/zstdmt_compress.c
+++ b/lib/compress/zstdmt_compress.c
@@ -141,6 +141,15 @@ typedef struct {
 
 /* assumption : CCtxPool invocation only from main thread */
 
+/* note : all CCtx borrowed from the pool should be released back to the pool _before_ freeing the pool */
+static void ZSTDMT_freeCCtxPool(ZSTDMT_CCtxPool* pool)
+{
+    unsigned u;
+    for (u=0; u<pool->availCCtx; u++)  /* note : availCCtx is supposed == totalCCtx; otherwise, some CCtx are still in use */
+        ZSTD_freeCCtx(pool->cctx[u]);
+    free(pool);
+}
+
 static ZSTDMT_CCtxPool* ZSTDMT_createCCtxPool(unsigned nbThreads)
 {
     ZSTDMT_CCtxPool* const cctxPool = (ZSTDMT_CCtxPool*) calloc(1, sizeof(ZSTDMT_CCtxPool) + nbThreads*sizeof(ZSTD_CCtx*));
@@ -149,10 +158,8 @@ static ZSTDMT_CCtxPool* ZSTDMT_createCCtxPool(unsigned nbThreads)
         for (threadNb=0; threadNb<nbThreads; threadNb++) {
             cctxPool->cctx[threadNb] = ZSTD_createCCtx();
             if (cctxPool->cctx[threadNb]==NULL) {   /* failed cctx allocation : abort cctxPool creation */
-                unsigned u;
-                for (u=0; u<threadNb; u++)
-                    free(cctxPool->cctx[u]);
-                free(cctxPool);
+                cctxPool->totalCCtx = cctxPool->availCCtx = threadNb;
+                ZSTDMT_freeCCtxPool(cctxPool);
                 return NULL;
     }   }   }
     cctxPool->totalCCtx = cctxPool->availCCtx = nbThreads;
@@ -165,7 +172,7 @@ static ZSTD_CCtx* ZSTDMT_getCCtx(ZSTDMT_CCtxPool* pool)
         pool->availCCtx--;
         return pool->cctx[pool->availCCtx];
     }
-    /* should not be possible, since totalCCtx==nbThreads */
+    /* note : should not be possible, since totalCCtx==nbThreads */
     return ZSTD_createCCtx();
 }
 
@@ -174,18 +181,10 @@ static void ZSTDMT_releaseCCtx(ZSTDMT_CCtxPool* pool, ZSTD_CCtx* cctx)
     if (pool->availCCtx < pool->totalCCtx)
         pool->cctx[pool->availCCtx++] = cctx;
     else
-    /* should not be possible, since totalCCtx==nbThreads */
+    /* note : should not be possible, since totalCCtx==nbThreads */
         ZSTD_freeCCtx(cctx);
 }
 
-static void ZSTDMT_freeCCtxPool(ZSTDMT_CCtxPool* pool)
-{
-    unsigned u;
-    for (u=0; u<pool->totalCCtx; u++)
-        ZSTD_freeCCtx(pool->cctx[u]);
-    free(pool);
-}
-
 
 struct ZSTDMT_CCtx_s {
     POOL_ctx* factory;