`pg-cursor` usage leads to memory leak

[viladimiru]

If you will execute any read query, you can see in node inspector that GC ignores query result and doesn't cleanup it

const client = new PostgreSqlClient({...credentials, query_timeout: 15 * 60_000});
const cursor = client.query(
    new Cursor(query),
    unhandledErrorCallback,
);
await new Promise((resolve, reject) => {
  cursor.read(100000, (error, _, result) => {
      if (error) {
          reject(error);
      } else {
          resolve(result);
      }
  })
})

This happens because of the callback with timeout cleanup: https://github.com/brianc/node-postgres/blob/master/packages/pg/lib/client.js#L577 query.callback will not be called even if you closed cursor and destroyed the client

[viladimiru]

Quick fix

const client = new PostgreSqlClient({...credentials, query_timeout: 15 * 60_000});
const cursor = client.query(
    new Cursor(query),
    queryCallback, // will be called in `cursor.callback()`
);
await new Promise((resolve, reject) => {
  cursor.read(100000, (error, _, result) => {
      if (error) {
          reject(error);
      } else {
          resolve(result);
      }
  })
})

// this way we call method, which responsible to clear timeout
// https://github.com/brianc/node-postgres/blob/master/packages/pg/lib/client.js#L576
cursor.callback();

[brianc]

hmmm I'm not sure mixing callbacks & promises here is the right approach. I've never seen a 2nd argument (callback) passed to client.query when a cursor is passed as the first argument. Sorry for being dumb about this but what are you trying to accomplish w/ that approach?

[viladimiru]

hmmm I'm not sure mixing callbacks & promises here is the right approach. I've never seen a 2nd argument (callback) passed to client.query when a cursor is passed as the first argument. Sorry for being dumb about this but what are you trying to accomplish w/ that approach?

Hello! Thank you for response.

Yes, it looks strange, I had to set callback to avoid problem with queryCallback is not a function error, more info described in the issue https://github.com/brianc/node-postgres/issues/1860.

And I had to use callback in cursor.read because I want to handle third argument, which is QueryResult, this structure is more appropriated to me, because it provides table fields, even if result is empty, sorry for the unnecessary details in example.

This is simplified example for reproduction:

const client = new PostgreSqlClient({...credentials, query_timeout: 15 * 60_000});
const cursor = client.query(new Cursor(query));
await cursor.read(100_000_000);