cmemory-leakszlibinflate

zlib inflateReset causes memory leak (not)


I am currently working on the below requirement. Here is the requirement: On the server side a large file is divided into 4000-byte blocks (frames). Each block is in turn compressed (using zlib) and sent to client process. For instance, if a file is 12000 bytes in size then it is divided into 3 blocks.

Above file will have 3 blocks => Block-0, Block-1, Block-2

On receipt, client decompresses each block (or frame) and writes to buffer allocated on the heap.When all the blocks corresponding to the entire file is received by the client, then the uncompressed version of the resultant file is written to the disk.

I have written a routine inflateData() that does the following based on the block # received:

When the first block is received,

With the above routine, Decompression of blocks happens as expected. But the issue that I face is it consumes lots of memory and at some point entire system slows down. When checked with valgrind, memory leak is reported with inflateInit2_. This causes the system resources to be exhausted.

==30359== 57,312 bytes in 6 blocks are possibly lost in loss record 64 of 67
==30359== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==30359== by 0x3E57808F1E: inflateInit2_ (in /lib64/libz.so.1.2.3)
==30359== by 0x40C220: inflateData (productMaker.c:1668)

Below is the inflateData() routine.

int inflateData(
    char*    const    inBuf,                  
    unsigned long     inLen,                  
    unsigned int      isFirstBlk,
    unsigned int      isLastBlk,
    const    char*    outBuf,                 
    unsigned long*    outLen)                 
{

  int have;
  int readsz;
  int bsize;
  static z_stream zstrm;
  int zerr;
  int flush;
  char out[CHUNK_SZ];
  char in[CHUNK_SZ];
  int ret,nwrite,idx = -1;
  int savedByteCntr=0;
  unsigned char *dstBuf;
  int  firstCall = 1;
  int totalBytesIn=0;
  int inflatedBytes=0;
  int decompByteCounter = 0;
  int num=0;


  ret = Z_OK;
  readsz = 0;
  bsize = CHUNK_SZ;

  dstBuf = (unsigned char *) outBuf;

  if(isFirstBlk){
        memset(&zstrm, '\0', sizeof(z_stream));
        zstrm.zalloc = Z_NULL;
        zstrm.zfree = Z_NULL;
        zstrm.opaque = Z_NULL;

        if ((zerr = inflateInit(&zstrm)) != Z_OK) {
                uerror("ERROR %d inflateInit (%s)",
                        zerr, decode_zlib_err(zerr));
                return -1;
        }
  }

    while(totalBytesIn < inLen ) {
      int compChunkSize = ((inLen - totalBytesIn) > 5120) ? 5120 :
                                                  (inLen - totalBytesIn);
      memcpy(in, inBuf + totalBytesIn, compChunkSize);

      zstrm.avail_in = inLen - totalBytesIn;
      zstrm.next_in = in ;

      zstrm.avail_out = CHUNK_SZ;
      zstrm.next_out = out;
      inflatedBytes = 0;

      while(ret != Z_STREAM_END) {
         ret  = inflate(&zstrm, Z_NO_FLUSH);
         if(ret < 0) {
          uerror(" Error %d inflate (%s)", ret, decode_zlib_err(ret));
          (void)inflateEnd(&zstrm);
           return ret;
         }
         inflatedBytes = CHUNK_SZ - zstrm.avail_out;

         if(inflatedBytes == 0) {
              unotice("\n Unable to decompress data - truncated");
              break;
         }
     totalBytesIn += zstrm.total_in;
     decompByteCounter += inflatedBytes;

     memcpy(dstBuf + savedByteCntr, out, inflatedBytes);
     savedByteCntr = decompByteCounter;
   }

   // Reset inflater for additional input
   ret = inflateReset(&zstrm);
   if(ret == Z_STREAM_ERROR){
      uerror(" Error %d inflateReset (%s)", ret, decode_zlib_err(ret));
      (void)inflateEnd(&zstrm);
       return ret;
   }
  }

if(isLastBlk){
 ret = inflateEnd(&zstrm);
 if(ret < 0) {
   uerror("Fail inflateEnd %d [%s] ", ret, decode_zlib_err(ret));
   return (ret);
 }
}

  *outLen = decompByteCounter;
   return 0;
}

Solution

  • You are making an error in your use of your inflateData() routine.

    First off, using a static variable in this way is a horrible idea. If you call your inflateData() twice with isFirstBlk true without an intermediate call with isLastBlk true, then you will wipe out the reference to the first set of allocations, resulting in a memory leak.

    To avoid this sort of error, you should keep track of whether zstrm is initialized or not, and reject any attempt to initialize an already initialized stream. Better still would be to not even have an isFirstBlk, and simply initialize zstrm on the first call and on any call that immediately follows a call with isLastBlk true.

    So you are either doing the above, calling twice with isFirstBlk true, or failing to call with isLastBlk true.