Skip to content
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

ReactiveSocket d'tor can be called from signal handler of a stream. #37

Merged
1 commit merged into from May 18, 2016
Merged

ReactiveSocket d'tor can be called from signal handler of a stream. #37

1 commit merged into from May 18, 2016

Conversation

ghost
Copy link

@ghost ghost commented May 17, 2016

DO NOT MERGE!

This is still a WIP, to do:

  • write a test to verify it actually works
  • provide stronger guarantees to DuplexConnection, so that it's easier to get right.

@ghost
Copy link
Author

ghost commented May 17, 2016

This fixes #34.

@ghost
Copy link
Author

ghost commented May 17, 2016

So far, I know that:

  • it's safe, since we hold a strong reference to ConnectionAutomaton, i.e. we cannot possibly dereference dangling pointer/reference,
  • ConnectionAutomaton doesn't hold strong references to AbstractStreamAutomata, so there is no leak -- I actually verified that d'tor of ConnectionAutomaton is invoked,
  • as a bonus, ConnectionAutomaton::endStream is still idempotent, which simplifies AbstractStreamAutomata.

@ghost
Copy link
Author

ghost commented May 18, 2016

GOOD TO MERGE (after review)

@yschimke, take a look at the test, please.

@yschimke
Copy link
Member

yschimke commented May 18, 2016

The test makes sense to me, and other changes all looked appropriate given the pointer changes.

@ghost ghost merged commit a68eee4 into rsocket:master May 18, 2016
@ghost ghost deleted the dtor branch May 18, 2016 00:35
facebook-github-bot pushed a commit that referenced this pull request Jul 24, 2019
Summary:
This is just facebookarchive/bistro#37, backported to `fbcode_builder`.

snarkmaster
Pull Request resolved: facebookarchive/bistro#37

Test Plan: Push and watch Travis

Reviewed By: simpkins

Differential Revision: D16453080

Pulled By: snarkmaster

fbshipit-source-id: a15eaead931f046c41e50f8e3b412ef68b172d65
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant