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.

Sign up for GitHub

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

Jump to bottom

Try to fix atexit not being called #6909

Merged
merged 1 commit into from
Jan 26, 2020
Merged

Try to fix atexit not being called #6909

merged 1 commit into from
Jan 26, 2020

Conversation

mehrdadn
Copy link
Contributor

@mehrdadn mehrdadn commented Jan 24, 2020 •
edited
Loading

Why are these changes needed?

atexit doesn't seem to be always called as of when we started using Boost.Process.

Related issue number

#631, #6510

Checks

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@mehrdadn mehrdadn force-pushed the boost-process branch 2 times, most recently from f4caea2 to 4c26cef Compare January 24, 2020 05:24
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21003/
Test FAILed.

@mehrdadn mehrdadn marked this pull request as ready for review January 24, 2020 08:23
@mehrdadn mehrdadn force-pushed the boost-process branch 2 times, most recently from 3b68463 to bdbea31 Compare January 24, 2020 08:35
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21006/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21009/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21018/
Test PASSed.

@mehrdadn mehrdadn force-pushed the boost-process branch 4 times, most recently from 3b68463 to 831e8f1 Compare January 25, 2020 02:36
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21031/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/21043/
Test FAILed.

edoakes
edoakes approved these changes Jan 26, 2020
View reviewed changes
Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

Re-ran tests and they LGTM, so merging

@edoakes edoakes merged commit bde575b into ray-project:master Jan 26, 2020
edoakes pushed a commit that referenced this pull request Jan 26, 2020
@mehrdadn @edoakes
Revert "Use Boost.Process instead of pid_t (#6510)" (#6909)
38ec2e7 
This reverts commit fb8e361.
@mehrdadn mehrdadn deleted the boost-process branch January 26, 2020 16:53
@mehrdadn mehrdadn mentioned this pull request Feb 11, 2020
Merged
mehrdadn pushed a commit to mehrdadn/ray that referenced this pull request Feb 17, 2020
@web-flow
Revert "Revert "Use Boost.Process instead of pid_t (ray-project#6510)" (
0b7c5bd 
ray-project#6909)"

This reverts commit bde575b.
pcmoritz pushed a commit that referenced this pull request Feb 19, 2020
@mehrdadn
Use Process instead of pid_t (round 2) (#6882)
4a12243 
* Revert "Revert "Use Boost.Process instead of pid_t (#6510)" (#6909)"

This reverts commit bde575b.

* Process wrapper, using Boost.Process on Windows

- Reverts bde575b.
- Re-applies fb8e361 after some refactoring.

* Remove Boost.Process dependency

* Don't open /proc file on Linux

* Change FATAL to ERROR and modify error message when process doesn't exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edoakes edoakes edoakes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mehrdadn @AmplabJenkins @edoakes @web-flow