Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removeListeners doesn't work as expected for newly created orders, causing callback leak #25

Open
ngutman opened this issue Aug 11, 2019 · 2 comments

Comments

@ngutman
Copy link

ngutman commented Aug 11, 2019

Summary

There's an issue causing removeListeners to not remove all the order callbacks that were registered in registerListeners causing a leak, as new orders accumulate this greatly affects performance eventually killing he process

Issue Breakdown

  • Upon initial creation of an order with cid the cbGID will be undefined.{cid} since gid was never provided
  • Calling registerListeners will register the mentioned cbGID which later is used with removeListeners
  • Submitting the order will invoke an update message updating the gid to null (we listen to update to verify the order was successfully submitted
  • cbGID will now be null.{cid} mismatching the initial cbGID, rendering removeListeners useless which will increase callbacks size infinitely - this specifically affects systems automatically creating orders.

Sample code

(I took the sample code from https://github.com/bitfinexcom/bfx-api-node-models/#order marking where the issue is prominent)

const { Order } = require('bfx-api-node-models')
const ws = ... // setup WSv2 instance for order updates/submission

// Build new order
const o = new Order({
  cid: Date.now(),
  symbol: 'tBTCUSD',
  price: 7000.0,
  amount: -0.02,
  type: Order.type.EXCHANGE_LIMIT
}, ws) // note WSv2 client passed in here

let closed = false

// Enable automatic updates
o.registerListeners()

o.on('update', () => {
  debug('order updated: %j', o.serialize())
  o.removeListeners() // ISSUE - `cbGID` is not the same as before the `update`
})

o.on('close', () => {
  debug('order closed: %s', o.status)
  closed = true
  o.removeListeners() // Probably also problematic
})

debug('submitting order %d', o.cid)

o.submit().then(() => {
  debug('got submit confirmation for order %d [%d]', o.cid, o.id)
}).catch((err) => {
  debug('failed to submit order: %s', err.message)
})
@pxr64
Copy link

pxr64 commented May 1, 2020

Anyway to get around this issue?

@hackable
Copy link

hackable commented Jan 23, 2021

removeListeners (apiInterface = this._apiInterface) {
    let cbGID = this.cbGID().replace('null','undefined');
    if (apiInterface) {
      apiInterface.removeListeners(cbGID)
    }
  }

this fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants