ccxtpro - memory leaks on workarounds for reinit websockets

[krychla1]

Hi fellows, currently you offer no possibility to recreate websockets connections/subscriptions in case they go wrong/dead. User workarounds have to be created, as you suggested - by creating new Exchange instances. However it is difficult/impossible, as the obsolete Exchange objects references cause memory leaks.

simple snippet:

class ExchangeWS {
    constructor(exchangeName, params) {
        this.exchangeName = exchangeName;
        this.params = params;
    }

    async init() {
        try {
            this.exchange = new ccxt[this.exchangeName](this.params);
            await this.exchange.loadMarkets(true);
            if (this.exchange.has.watchBalance) {
                await this.exchange.watchBalance();
            }
            this.ready = true;
        } catch (e) {
            console.log(`${this.exchangeName} | init | ${e.stack}`);
        }
    }

    async reinit() {
        try {
            console.log(`${this.exchangeName} | reinit`);
            this.ready = false;
            // await this.delete_wsClients();   //doesn't help
            await this.exchange.sleep(1000);
            delete this.exchange;
            await this.init();
        } catch (e) {
            console.log(`${this.exchangeName} | reinit | ${e.stack}`);
        }
    }

    // tried to remove any event/connection before reinit, no success
    async delete_wsClients() {
        try {
            if (!this.exchange.clients) {
                console.log(`${this.exchangeName} | no clients`);
                return;
            }
            for (let client of Object.keys(this.exchange.clients)) {
                if (this.exchange.clients[client].connection) {

                    for (let event of Object.keys(this.exchange.clients[client].connection._events)) {
                        await this.exchange.clients[client].connection.removeEventListener(event, this.exchange.clients[client].connection._events[event]);
                    }
                    // await this.exchange.clients[client].connection.emitClose();
                    if (this.exchange.clients[client]?.connection)
                        await this.exchange.clients[client].connection.close();
                }
                if (this.exchange.clients[client]?.close)
                    await this.exchange.clients[client].close();
                delete this.exchange.clients[client];
            }
            await this.exchange.sleep(1000);

        } catch (e) {
            console.log(`${this.exchangeName} | delete_wsClients: ${e.stack}`);
        }
    }


}

async function test() {
    try {
        let myExchange = new ExchangeWS(exchangeName, params);
        await myExchange.init();
        
        while (true) {
            let book = await myExchange.exchange.watchOrderBook('BTC/USDT');
            console.log(book.bids.length + '/' + book.asks.length);
            await myExchange.exchange.sleep(2000);
            await myExchange.reinit();
        }

    } catch (e) {
        console.log(`test: ${e}`);
    }
}

test()

Please take a look. Thanks.

[ttodua]

This might need a bit rework in CCXT base class to address that issue. Devs will review and post an update whenever possible. In the meanwhile I might suggest to try removing all event listeners & nullify the connection object itself in the end, like `

this.exchange.clients[client].connection.removeAllListeners();
this.exchange.clients[client].connection=null;

and let us know if that helps or not in the meanwhile.

---

Also, I've checked in client.js and also there is reset() function you might try inside your disconnection codes:

if (this.exchange.clients[client].connection?.terminate)
                    await this.exchange.clients[client].connection.terminate();

and this.exchange.clients[client].reset( new ccxt.NetworkError ("temp disconnetion") )

which in my opinion, (as I see with quick glance) should remove all eventhandlers tied to that client.

( you can find the futher examples here too about disposing WS clients: https://stackoverflow.com/a/49791634/2377343 )

[krychla1]

@ttodua you are a sniper!! The intervals and timeouts (in client.js) were keeping all the references in memory. To clear them I picked these functions from the client.reset():

this.exchange.clients[client].clearConnectionTimeout();
this.exchange.clients[client].clearPingInterval();

Seems working so far. Thank you!

[ttodua]

@kroitor
What do you think about memory-management built-in functionality, can we create an inner function which will help to mitigate those scenarios? I think, memory leakage might be a considerable problem for dependent entities (also, the remnants of the previously initialized WebSocket exchange object, might be taking some resources..). Not all people can do like this OP made a custom code to solve the problem.

I think there might be two ways:

• A) create destroy (or destruct or alike) where it will do like reset() but will also nulify/destroy all other things too, but that destroy() function should live inside exchange-base class, and should be called explicitly by user, while he wants to dispose initialized exchange:

   let ex = new ccxtpro.XYZexchange({}.
   ... await ex.watchSmth();
   ...
   ...
   // Task done, we are closing our script
   ex.destroy();

Where destroy() function will include all necessary things (including something like OP did):

      destroy(){
           ...  
           for( let client in clients){
                  this.reset(); 
           }
           ...
           all  other needed things
       }

• B) create inner timer-functionality, which will be called internally (i.e. 1000-5000 MS interval) checking if the exchange object is still alive or not. and in case it is no longer used (or nulified), like:

  let ex = new ccxtpro.XYZexchange({}.
  ... await ex.watchSmth();
  ...
  // Task done, we are closing our script
  ex = null;  //or   'delete ex';

so internal function was in Exchange.js base inside constructor like:

    selfReferencedInstance : null,

    constructor () { 
        ...
        selfReferencedInstance  = this;
        setInterval( this.checkForDisposal, 1000);
       ...
    }

    checkForDisposal(){
        if ( ! selfReferencedInstance ){
           this.destroy();
        }
    }
  

something like that, what do you think. Even though (A) version to use ex.destroy() is more direct way, still because of imperfection of userland codes, something (B) (but more sophisticated approach) might help too.

[kroitor]

@ttodua personally, i think the user should not have to mess with the ws client internals of course. I think we should find the precise cause of a memleak and fix it in all such cases. Since there's a call to a destructor in the user's code, we should make sure that it destroying an exchange leaves no trace behind it.

[krychla1]

From my point of view the best way would be the combination of both of your ideas. The user should not mess with the inner workings of your framework to prevent undesired/unsafe behavior.

It would be useful to have a framework functions to close/reinit the WS or its subscriptions, that would be properly handled by your code (kind of A) of ttoduas) so that users have no incentives to destroy the exchange object at all.
(currently we have to, because several exchanges in ccxt.pro are unreliable and there is no possibility out of the box to check/reinit them)

[Onaxim]

Has anyone found a solution to the memory leaks when reinstantiating an instance of an exchange?

Tried clearing client intervals with below but no avail.

this.exchange.clients[client].clearConnectionTimeout();
this.exchange.clients[client].clearPingInterval();

[goznauk]

Are there any updates?