add support for os.fork and related functions#4877
add support for os.fork and related functions#4877youknowone merged 67 commits intoRustPython:mainfrom
Conversation
vm/src/stdlib/os.rs
Outdated
|
|
||
| #[pyfunction] | ||
| fn fork(vm: &VirtualMachine) -> PyObjectRef { | ||
| unsafe { vm.ctx.new_int(libc::fork()).into() } |
There was a problem hiding this comment.
So, slightly looking into this it seems that you'll need to do a couple things:
- Add the
register_at_forkfunction. This does seem to be slightly straight-forward, logic wise at lease. You take in three callables and, after some sanity checks, store them in the interpreter state (typicallyVirtualMachine'sPyGlobalStatein our case). This can ideally be a struct but using a vec a-laatexit_funcscould be another approach if thestructis stopping you. - Modify
forkto use these. Looking at CPython's implementation this again seems relatively straight-forward. Callbeforebefore you issue theforkcall and based on if you're the child/parent (pid == 0) issue eitherafter_in_child/after_in_parent.
This is really a very high level overview since I do not have the time to spare. Most likely some other thing might also need to be done and I'm just not seeing it now.
There was a problem hiding this comment.
Thanks, Point 2 looks easy but I was stuck point 1, but now I can move ahead with this pointer.
|
Okay, this is functional now, code snippet: import os
def before():
print("before")
def parent():
print("parent")
def child():
print("Child")
os.register_at_fork(before=before, after_in_child=child, after_in_parent=parent)
os.fork()in rustpython In cpython ( 3.12.0a7+) wrt to error handling: rustpython cpython Some obvious review changes are needed for:
Pending: test cases
in test_waitpid, it shows # TODO: RUSTPYTHON (AttributeError: module 'os' has no attribute 'spawnv') |
|
Nice start! Some initial things so we get a better picture.
|
Co-authored-by: fanninpm <[email protected]>
|
I suspect something in |
Co-authored-by: Jeong, YunWon <[email protected]>
Co-authored-by: Jeong, YunWon <[email protected]>
Co-authored-by: Jeong, YunWon <[email protected]>
Thanks for looking this up. I knew that would be error but couldn't see it and hence couldn't add skip tag.
Yes, but I don't know why it didn't got captured at output I even added sleep(60) at child so that I could send all chunk of traceback. Probably, just UI issue on github action as it worked fine for you. |
|
I have made required changes (adding missing XXX RustPython and using #[derive(FromArgs)]) |
| @unittest.skipIf(_testcapi is None, 'needs _testcapi') | ||
| @unittest.skipUnless(sys.platform in ("linux", "darwin"), | ||
| "Only Linux and macOS detect this today.") | ||
| def test_fork_warns_when_non_python_thread_exists(self): |
There was a problem hiding this comment.
is this originated from CPython source code?
There was a problem hiding this comment.
Yes
@unittest.skipUnless(sys.platform in ("linux", "darwin"),
"Only Linux and macOS detect this today.")
def test_fork_warns_when_non_python_thread_exists(self):
...There was a problem hiding this comment.
Ah, got it. it came from later then 3.11.
vm/src/stdlib/os.rs
Outdated
|
|
||
| #[cfg(unix)] | ||
| fn py_os_before_fork(vm: &VirtualMachine) -> PyResult<()> { | ||
| let before_forkers: Vec<PyObjectRef> = std::mem::take(&mut *vm.state.before_forkers.lock()); |
There was a problem hiding this comment.
Is this not clone but take? So that second run of fork doesn't be affect by it?
There was a problem hiding this comment.
Actually you are right.
import os
def a():
print('a')
def b():
print('b')
def c():
os.register_at_fork(before=a)
os.fork()
os.register_at_fork(before=b)
os.fork()
c()yields
a
b
a
b
a
in cpython. I have remove take operation and now rustpython as same results.
This should be a test case in cpython.
Co-authored-by: Jeong, YunWon <[email protected]>
Co-authored-by: Jeong, YunWon <[email protected]>
|
Made required changes, optional arg is working fine as well. working without any issue |
Co-authored-by: Jeong, YunWon <[email protected]>
youknowone
left a comment
There was a problem hiding this comment.
Thank you! You made a big step on fork.
I wonder how many more features needed for gunicorn.
Thanks a lot for review and helping me with this PR. |
Motivation: I was trying to run small python server using gunicorn but it failed giving me error fork is not found in module os. This motivated me to add this feature because a running a web server is big deal (for me, atleast)
This patch add fork via libc and test cases from test_os.py
Output is as follows:
In cpython
Current challenges:
implementation of function register_at_fork
I am not entirely sure how to translate cpython's os_register_at_fork_impl to rustpython (suggestions are strongly welcome).