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

Prevent creating multiple Clients or Media Drivers with the same cont… #666

Merged
merged 1 commit into from May 12, 2019

Conversation

mikeb01
Copy link
Contributor

@mikeb01 mikeb01 commented May 12, 2019

…ext. Allowing it with clients cause a SEGV, instead now will throw IllegalStateException. This has tripped me up twice now.

…ext. Allowing it with clients cause a SEGV, instead now will throw IllegalStateException.
@mjpt777 mjpt777 merged commit dde01f2 into real-logic:master May 12, 2019
mjpt777 added a commit that referenced this pull request May 12, 2019
…e on solution provided in PR #666, increase minor version as a result of change.
@mikeb01
Copy link
Contributor Author

mikeb01 commented May 12, 2019

I notice that you moved the implementation inside of conclude. This is what I had originally, but I found that this implementation still segfaults as the the Aeron.connect constructs the Aeron instance inside of a try/catch block that closes the reused context on failure.

@mjpt777
Copy link
Contributor

mjpt777 commented May 12, 2019

Good point. Maybe a different exception type that does not result in close being called.

mjpt777 added a commit that referenced this pull request May 12, 2019
@mikeb01
Copy link
Contributor Author

mikeb01 commented May 12, 2019

I'll have a look today.

@mjpt777
Copy link
Contributor

mjpt777 commented May 12, 2019

I pushed a change that does this.

@mikeb01
Copy link
Contributor Author

mikeb01 commented May 12, 2019

Looks good. Added another small PR that includes the unit tests.

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.

None yet

2 participants