ARROW-1265: [Plasma] Clean up all resources on SIGTERM to keep valgrind output clean #921
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change-Id: I8c4fc9dc9a4b1b0e6eab44b912c92757eb2a7e43
Change-Id: I81d5f8e11709d9ae168ac074b6a229f566d56f74
pcmoritz
reviewed
Jul 31, 2017
cpp/src/plasma/events.h
Outdated
| /// \brief Run the event loop. | ||
| /// | ||
| /// @return Void. | ||
| void run(); |
Contributor
There was a problem hiding this comment.
Consider renaming this to EventLoop::Start for overall consistency with EventLoop::Stop
Member
Author
There was a problem hiding this comment.
Sounds good. I made a clean sweep over the rest of the EventLoop methods for consistency
Contributor
|
Except for the small inconsistency, this looks good to me! |
Change-Id: Ie38b9086d2b55e1c97b22f731276a94038e50e3a
Member
Author
|
Appveyor build running here, will merge when complete: https://ci.appveyor.com/project/wesm/arrow/build/1.0.811 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@pcmoritz this might be a little bit OCD since all of the valgrind warnings were for memory that was still reachable, but let me know what you think.