JCMais / node-libcurl

libcurl bindings for Node.js
https://npmjs.org/package/node-libcurl
MIT License
666 stars 118 forks source link

memory leak when using XFERINFOFUNCTION #381

Open tobiasklevenz opened 1 year ago

tobiasklevenz commented 1 year ago

Hey there, we noticed a memory leak when using XFERINFOFUNCTION.

We use the following code in a worker process running in an ec2 instance that receives a job with a url from a queue and fetches it:


function curl(
  url: string
): Promise<{ status: number; text: string; ok: boolean }> {
  return new Promise((resolve, reject) => {
    const curl = new Curl();

    curl.setOpt(Curl.option.URL, url);
    curl.setOpt(Curl.option.ACCEPT_ENCODING, "gzip,deflate");
    curl.setOpt(Curl.option.FOLLOWLOCATION, 1);

    curl.setOpt(Curl.option.NOPROGRESS, 0);
    // abort request if larger then MAX_DL_SIZE
    curl.setOpt(Curl.option.XFERINFOFUNCTION, (dltotal) => {
      return dltotal >= MAX_DL_SIZE ? 1 : 0;
    });

    curl.on("end", (status, data) => {
      curl.close();
      resolve({
        status,
        text: data.toString(),
        ok: status >= 200 && status < 300,
      });
    });

    curl.on("error", (e, curlCode) => {
      curl.close();
      reject(
        curlCode === CurlCode.CURLE_ABORTED_BY_CALLBACK
          ? new Error(`dl bigger than ${MAX_DL_SIZE}`)
          : e
      );
    });

    curl.perform();
  });
}

Once we introduced the XFERINFOFUNCTION code to limit the download size of the request, we saw a significant increase in memory usage, which over the course of a few hours completely maxed out our instances memory. We tried adding curl.enable(CurlFeature.Raw | CurlFeature.NoStorage) and handling the received data in a separate writefunction but got the same results.

To fix the issue we switched to implementing WRITEFUNCTION and handle the dl size limit there:

    curl.setOpt(Curl.option.WRITEFUNCTION, (chunk) => {
      data = Buffer.concat([data, chunk]);

      if (data.length >= MAX_DL_SIZE) {
        return 0;
      }

      return chunk.length;
    });

Here's a screenshot of our memory usage showing before and after we switched to the writefunction:

image