search

dominictarr / JSONStream

rawStream.pipe(JSONStream.parse()).pipe(streamOfObjects)
Other
1.92k stars 165 forks source link

fix memory leak problem when streaming huge files #154

Closed daern91 closed 6 years ago

daern91 commented 6 years ago

PR reverts commits 2e4fcb and 8c1206 that introduced memory leaks.

Please squash merge.

resolves #153

dominictarr commented 6 years ago

@mfogel looks like this change will break your code?

mfogel commented 6 years ago

@dominictarr yeah I think so. This would re-open #148

Seems that it should be possible to fix this memory leak issue without re-breaking the double-dot operator.

daern91 commented 6 years ago

My bad, I missed the earlier issue. Anyways, thanks guys, @dominictarr and @mfogel. Updated the PR now to fix memory leak without reverting @mfogel's fixes.

daern91 commented 6 years ago

Any update here @dominictarr?

ruettenm commented 6 years ago

It would be nice to merge this PR :-)

daern91 commented 6 years ago

@dominictarr @mfogel anything else you can see that is blocking this PR? Quite a few people that would appreciate a fix https://github.com/dominictarr/JSONStream/issues/153

mfogel commented 6 years ago

As far as I can see, the PR now looks ok to me.

dominictarr commented 6 years ago

@daern91 hey, sorry for the delay. would you be willing to help maintain this module? I don't really even use it anymore, and have lots of other things to do with my time. I can add you and you can merge this

daern91 commented 6 years ago

@dominictarr No problems at all, I don't have time for very active maintenance but can definitely read through PRs, fix smaller issues and merge as needed.

dominictarr commented 6 years ago

@daern91 there arn't many issues/PRs, it's just that I have loads of other stuff

dominictarr commented 6 years ago

@daern91 you can now merge and publish