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

fix: stream.cancel were causing double-free and segfault #328

Merged
merged 7 commits into from
Oct 4, 2021

Conversation

jawid-h
Copy link
Contributor

@jawid-h jawid-h commented Sep 30, 2021

Issue being fixed or feature implemented

Due to a bug in grpc-js explained here. A temporary solution is used to bypass this bug until a fix is merged into Node.js explained here

What was done?

  • wrapped up stream.cancel into setImmediate

How Has This Been Tested?

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@jawid-h jawid-h added this to the v0.21.0 milestone Sep 30, 2021
Alex-Werner
Alex-Werner previously approved these changes Sep 30, 2021
Copy link
Contributor

@Alex-Werner Alex-Werner left a comment

Choose a reason for hiding this comment

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

Wow ! I didn't noticed nor expected that one.
Very good catch ! LGTM !

shumkov
shumkov previously approved these changes Sep 30, 2021
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

Good catch! 👍

@jawid-h jawid-h dismissed stale reviews from shumkov and Alex-Werner via ee3989c September 30, 2021 14:47
@Alex-Werner Alex-Werner force-pushed the fix/grpc-double-free branch 2 times, most recently from e15407e to ee3989c Compare October 1, 2021 00:06
.github/workflows/.env Outdated Show resolved Hide resolved
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

Good catch! 👍

Copy link
Contributor

@Alex-Werner Alex-Werner left a comment

Choose a reason for hiding this comment

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

LGTM while we get back at it

@jawid-h jawid-h merged commit b90644f into v0.21-dev Oct 4, 2021
@jawid-h jawid-h deleted the fix/grpc-double-free branch October 4, 2021 16:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants