From 8afb151c9b0ddb102cee7637b22552fec1d6acb0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 29 Sep 2017 22:14:37 -0700 Subject: [PATCH] cli: fixed wrong initialization in MT mode It's not good to mix old and new API ZSTD_resetCStream() doesn't just set pledgedSrcSize : it also sets the CCtx for a single thread compression. Problem is, when 2+ threads are defined in cctx->requestedParams, ZSTD_compress_generic() will want to start MT compression, since initialization is supposed to have already happened (thanks to ZSTD_resetCStream()) except that the underlying ZSTDMT_CCtx* object is not created, resulting in a segfault. This is an invalid construction (correct one is to use ZSTD_CCtx_setPledgedSrcSize()). I haven't found a nice way to mitigate this impact if someone makes the same mistake. At some point, removing the old API to keep only the new API within fileio.c will limit these risks. --- lib/compress/zstd_compress.c | 3 ++- programs/fileio.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 6f369eba..7fccc180 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2756,6 +2756,7 @@ size_t ZSTD_compress_generic (ZSTD_CCtx* cctx, ZSTD_inBuffer* input, ZSTD_EndDirective endOp) { + DEBUGLOG(5, "ZSTD_compress_generic"); /* check conditions */ if (output->pos > output->size) return ERROR(GENERIC); if (input->pos > input->size) return ERROR(GENERIC); @@ -2798,7 +2799,7 @@ size_t ZSTD_compress_generic (ZSTD_CCtx* cctx, #ifdef ZSTD_MULTITHREAD if (cctx->appliedParams.nbThreads > 1) { size_t const flushMin = ZSTDMT_compressStream_generic(cctx->mtctx, output, input, endOp); - DEBUGLOG(5, "ZSTDMT_compressStream_generic : %u", (U32)flushMin); + DEBUGLOG(5, "ZSTDMT_compressStream_generic result : %u", (U32)flushMin); if ( ZSTD_isError(flushMin) || (endOp == ZSTD_e_end && flushMin == 0) ) { /* compression completed */ ZSTD_startNewCompression(cctx); diff --git a/programs/fileio.c b/programs/fileio.c index 6dc43de8..1319a575 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -446,6 +446,7 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_targetLength, comprParams->targetLength) ); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_compressionStrategy, (U32)comprParams->strategy) ); /* multi-threading */ + DISPLAYLEVEL(5,"set nb threads = %u \n", g_nbThreads); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_nbThreads, g_nbThreads) ); /* dictionary */ CHECK( ZSTD_CCtx_loadDictionary(ress.cctx, dictBuffer, dictBuffSize) ); @@ -797,7 +798,8 @@ static int FIO_compressFilename_internal(cRess_t ress, /* init */ #ifdef ZSTD_NEWAPI - CHECK( ZSTD_resetCStream(ress.cctx, fileSize) ); /* to pass fileSize */ + if (fileSize!=0) /* if stdin, fileSize==0, but is effectively unknown */ + ZSTD_CCtx_setPledgedSrcSize(ress.cctx, fileSize); #elif defined(ZSTD_MULTITHREAD) CHECK( ZSTDMT_resetCStream(ress.cctx, fileSize) ); #else