Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

fix _scratch 'leak' to global/window #42

Merged
merged 1 commit into from
Apr 26, 2018
Merged

Conversation

Xcrux
Copy link
Contributor

@Xcrux Xcrux commented Apr 11, 2018

_scratch was leaking because checkpointInterface was called without assigning this (in strict mode it would fail because this would be undefined, otherwise it takes global/window as 'this', hence the leak)
I'm not familiar with tape, if there's an option to check leaks (like --check-leaks in mocha) it would be great to use - it will never happen again in the future. I didn't find something similar to that in tape.

function checkpointInterface (trie) {
  this._scratch = null // before the fix `this` may be global, window or undefined
  // ...
}

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.135% when pulling 2818d8a on Xcrux:master into 67fb885 on ethereumjs:master.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, I would have never seen this! Not sure about directly testing this with tape.

@holgerd77 holgerd77 merged commit 75dff20 into ethereumjs:master Apr 26, 2018
dekz added a commit to 0xProject/0x-monorepo that referenced this pull request Nov 20, 2019
2.3.1 has an issue which results in failure when ran in the browser with strict mode enabled. See ethereumjs/merkle-patricia-tree#42 for more details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants