-
Notifications
You must be signed in to change notification settings - Fork 462
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
fix(@libp2p/webtransport): maximum call stack size exceeded on abort #1947
Conversation
Fixes bug in webtransport where the `.abort` method on the stream class was calling `stream.abort` and leading to a call stack overflow.
writer.abort() | ||
writerClosed = true | ||
} | ||
stream.abort(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where the call stack overflow was happening.
@achingbrain I am not familiar with this commit convention, where is it derived from? |
Do you mean the |
It's hard to fit the scope into the git commit and still be under 50 characters, but with modern technology, I prefer scoped commits rather than short git commit messages. |
export function isSubset (set: Uint8Array[], maybeSubset: Uint8Array[]): boolean { | ||
const intersection = maybeSubset.filter(byteArray => { | ||
return Boolean(set.find((otherByteArray: Uint8Array) => uint8ArrayEquals(byteArray, otherByteArray))) | ||
}) | ||
return (intersection.length === maybeSubset.length) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this to be Uint8array or can it be generic?
Fixes bug in webtransport where the
.abort
method on the stream class was callingstream.abort
and leading to a call stack overflow.Also splits the code into multiple files to make it a bit easier to see what is doing what.