Fewbytes / rubber-docker

A workshop on Linux containers: Rebuild Docker from Scratch
MIT License
2.99k stars 237 forks source link

Add system exit when leaving child process. #11

Closed maximveksler closed 8 years ago

maximveksler commented 8 years ago

The thing fork now creates 2 flows in your code, so when blocking contain() returns, you will continue executing code. In this case no code is present post the if branch but should someone add a log / cleanup code / whatever that would run from both the parent and the child code flows. See here for better explanation of the same point http://www.python-course.eu/forking.php

nukemberg commented 8 years ago

that's a good point. however, in python one should use sys.exit(0). Mind fixing your pull request?

maximveksler commented 8 years ago

@avishai-ish-shalom are you sure about that? https://docs.python.org/2/library/os.html#os._exit

Note The standard way to exit is sys.exit(n). _exit() should normally only be used in the child process after a fork().

natict commented 8 years ago

Note that in an error-less flow, the added os._exit() is unreachable (since we are exec-ing at the end of contain). I'd go with: try: contain(...) finally: os._exit(1) # something failed in contain if we reached here

maximveksler commented 8 years ago

Where? https://github.com/Fewbytes/rubber-docker/search?utf8=✓&q=exit

natict commented 8 years ago

After adding the missing code, the contain() function should end with some variant of exec (you can find it in all the other levels).

maximveksler commented 8 years ago

Good point, you're right. So should we update the PR with try: finally or leave it as it is for the sake of clarity of being an example?