Max Call Stack Exceeded when removing patterns

[DonovanBoddy]

The max call stack error that was closed in v4.1.1 is still occurring.

It can be reproduced using the following code :

Node v-8.11.1 OS - Windows 10 Bloomrun v-4.1.1

const bloomrun = require('bloomrun')()
const noop = () => 'test'
for (let index = 0; index < 5000; index++) {
    bloomrun.add({ topic: 'math', cmd: `add${index}` }, noop)
}
bloomrun.list(null, {
    patterns: true
}).forEach(function (element) {
    bloomrun.remove(element)
})

Error -

D:\Workspace\Node.js\DummyServer\node_modules\bloomrun\lib\bucket.js:29
Bucket.prototype.remove = function (pattern, payload) {
                                   ^
RangeError: Maximum call stack size exceeded                                \lib\bucket.js:29:36)
    at Bucket.remove (D:\Workspace\Node.js\DummyServer\node_modules\bloomrun\lib\bucket.js:40:14)\lib\bucket.js:29:36)  

[mcollina]

Can you send a PR to fix?

Il giorno lun 7 gen 2019 alle 13:18 DonovanBoddy [email protected] ha scritto:

The max call stack error that was closed in v4.1.1 is still occurring.

It can be reproduced using the following code :

Node v-8.11.1 OS - Windows 10 Bloomrun v-4.1.1

const bloomrun = require('bloomrun')() const noop = () => 'test' for (let index = 0; index < 1000; index++) { bloomrun.add({ topic: 'math', cmd: add${index} }, noop) } bloomrun.list(null, { patterns: true }).forEach(function (element) { bloomrun.remove(element) })

Error -

D:\Workspace\Node.js\DummyServer\node_modules\bloomrun\lib\bucket.js:29 Bucket.prototype.remove = function (pattern, payload) { ^ RangeError: Maximum call stack size exceeded \lib\bucket.js:29:36) at Bucket.remove (D:\Workspace\Node.js\DummyServer\node_modules\bloomrun\lib\bucket.js:40:14)\lib\bucket.js:29:36)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mcollina/bloomrun/issues/69, or mute the thread https://github.com/notifications/unsubscribe-auth/AADL41M9m98ZVl9-iiBVlXyK5VTxbWn2ks5vAzsYgaJpZM4ZzZuz .

[DonovanBoddy]

I'm sorry but I don't have a fix, was hoping someone more skilled in node/javascript would be able to solve this issue

[gabssnake]

Even if you avoid recursion which is exceeding the stack, it takes about 30s to process 5k entries, and after 10k entries or so, there’s a memory crash.

Also, the issue is not present when patterns differ completely, e.g.:

// Performs well
{ topic:`math${index}`, cmd: `add${index}` }

Here’s one (silly?) way to avoid exceeding the stack while still passing all tests (more tests needed).

--- a/bloomrun.js
+++ b/bloomrun.js
@@ -83,11 +83,11 @@ Bloomrun.prototype.remove = function (pattern, payload) {
         if (this._tree[key] && this._tree[key][pattern[key]]) {
           bucket = this._tree[key][pattern[key]]
-          if (bucket.remove(pattern, payload)) {
+          while (bucket.remove(pattern, payload)) {
             removeBucket(this._buckets, bucket)
             delete this._tree[key][pattern[key]]
-            bucket.forEach(addPatternSet, this)
           }
+          bucket.forEach(addPatternSet, this)
         }
       }
     }

--- a/lib/bucket.js
+++ b/lib/bucket.js
@@ -27,23 +27,16 @@ Bucket.prototype.add = function (set) {
 }

 Bucket.prototype.remove = function (pattern, payload) {
-  var foundPattern = false
   var data = this.data
   var justRegex = onlyRegex(pattern)
   for (var i = 0; i < data.length; i++) {
     if (match(pattern, data[i].pattern)) {
       if (payload === null || payload === data[i].payload || justRegex) {
         data.splice(i, 1)
-        foundPattern = true
-        // to remove all occurences
-        this.remove(pattern, payload)
-        break
+        return true
       }
     }
   }
-
-  return foundPattern
 }

Test case running v-e-e-ery slow (30s):

test('issue#69 - no stack exceeded when removing similar patterns', function (t) {
  t.plan(1)
  var instance = bloomrun()
  var noop = () => 'test'
  for (let index = 0; index < 5000; index++) {
    instance.add({ topic: `math`, cmd: `add${index}` }, noop)
  }
  instance
    .list(null, { patterns: true })
    .forEach(pattern => instance.remove(pattern))
  t.equal(instance.list().length, 0)
})

[mcollina]

Would you like to send a PR?

[gabssnake]

Hi Matteo, do you have any pointers?

[mcollina]

Not really, I'm not using this anymore :/.

[StarpTech]

@mcollina bloomrun has dependents on:

• https://github.com/upringjs/upring
• https://github.com/mcollina/autocannon-ci

They aren't maintained anymore?