tahoe-lafs/trac-2024-07-25

remove exec() from the codebase #1651

New issue

Closed

opened 2012-01-06 00:10:56 +00:00 by warner · 2 comments

warner commented

2012-01-06 00:10:56 +00:00

Owner

As I remember, fork+exec came up as a way to make tahoe start cleanly daemonize, by executing twistd as a child process. Since then, I've figured out ways to use twistd's own code for this purpose, starting with an import and ending with a function call (which never returns, because the twistd code invokes fork and then os._exit() the parent).

To keep tahoe start from killing off the trial process during unit tests, we can do a fork() inside tahoe start before calling twistd. We should investigate how well this works on windows (I think we might skip that test on windows anyways).

I think this would let us get rid of a lot of code, followed by removing a lot of setuptools-ish code (the stuff that touches os.environPYTHONPATH and os.environPATH, and the tests that assert things about the version of code found by the tahoe start child). And that might let us reduce our dependency on setuptools and versioning stuff in general, at least a little bit.

We've had a number of "what version of the code are we running anyways?" problems, provoking tremendous heroics in the setuptools-based dependency-version-management code, that could go away entirely if we got rid of the use of `exec()` in the codebase (reducing the use of `fork()` would be good too, but not as critical). As I remember, fork+exec came up as a way to make `tahoe start` cleanly daemonize, by executing `twistd` as a child process. Since then, I've figured out ways to use twistd's own code for this purpose, starting with an import and ending with a function call (which never returns, because the twistd code invokes fork and then `os._exit()` the parent). To keep `tahoe start` from killing off the trial process during unit tests, we can do a `fork()` inside `tahoe start` before calling twistd. We should investigate how well this works on windows (I think we might skip that test on windows anyways). I think this would let us get rid of a lot of code, followed by removing a lot of setuptools-ish code (the stuff that touches `os.environPYTHONPATH` and `os.environPATH`, and the tests that assert things about the version of code found by the `tahoe start` child). And *that* might let us reduce our dependency on setuptools and versioning stuff in general, at least a little bit.

tahoe-lafs added the

labels 2012-01-06 00:10:56 +00:00

tahoe-lafs added this to the eventually milestone 2012-01-06 00:10:56 +00:00

davidsarah commented

2012-01-06 01:43:47 +00:00

Author

Owner

Replying to warner:

We've had a number of "what version of the code are we running anyways?" problems, provoking tremendous heroics in the setuptools-based dependency-version-management code, that could go away entirely if we got rid of the use of exec() in the codebase (reducing the use of fork() would be good too, but not as critical).

It's not as simple as that; if we don't set PYTHONPATH then we need to munge sys.path instead. As far as I can see this doesn't by itself do anything to solve any of the related bugs.

It does make it possible to then munge sys.path differently, and more correctly, to how setuptools does it. But don't underestimate the amount of work needed to do that right.

To keep tahoe start from killing off the trial process during unit tests, we can do a fork() inside tahoe start before calling twistd. We should investigate how well this works on windows (I think we might skip that test on windows anyways).

No, almost all of the subprocess tests work on Windows (and there are a lot of them).

I don't see the advantage of not creating subprocesses in tests. It doesn't simplify anything as far as I can see, and if it means we are doing something different on Unix than on Windows, it just makes things more complicated. (It might improve the speed of the relevant tests slightly, but I'm not sure that would be a significant proportion of the total time.)

Replying to [warner](/tahoe-lafs/trac-2024-07-25/issues/9126): > We've had a number of "what version of the code are we running anyways?" problems, provoking tremendous heroics in the setuptools-based dependency-version-management code, that could go away entirely if we got rid of the use of `exec()` in the codebase (reducing the use of `fork()` would be good too, but not as critical). It's not as simple as that; if we don't set PYTHONPATH then we need to munge sys.path instead. As far as I can see this doesn't by itself do anything to solve any of the related bugs. It *does* make it possible to then munge sys.path differently, and more correctly, to how setuptools does it. But don't underestimate the amount of work needed to do that right. > To keep `tahoe start` from killing off the trial process during unit tests, we can do a `fork()` inside `tahoe start` before calling twistd. We should investigate how well this works on windows (I think we might skip that test on windows anyways). No, almost all of the subprocess tests work on Windows (and there are a lot of them). I don't see the advantage of not creating subprocesses in tests. It doesn't simplify anything as far as I can see, and if it means we are doing something different on Unix than on Windows, it just makes things more complicated. (It might improve the speed of the relevant tests slightly, but I'm not sure that would be a significant proportion of the total time.)

exarkun commented

2020-01-13 20:44:49 +00:00

Author

Owner

tahoe start is deprecated and I'm not sure what other exec-family (execv, execl, etc) function usage this ticket is aimed at (note: this is not about built-in Python exec).

Closing. New tickets can be opened if people come across specific remaining uses of exec-family functions.

`tahoe start` is deprecated and I'm not sure what other `exec`-family (execv, execl, etc) function usage this ticket is aimed at (note: this is not about built-in Python exec). Closing. New tickets can be opened if people come across specific remaining uses of `exec`-family functions.

tahoe-lafs added the

cannot reproduce

label 2020-01-13 20:44:49 +00:00

exarkun closed this issue

2020-01-13 20:44:49 +00:00