python / cpython

The Python programming language
https://www.python.org
Other
62.51k stars 30.01k forks source link

Allow registering at-fork handlers #60704

Closed tiran closed 7 years ago

tiran commented 11 years ago
BPO 16500
Nosy @malemburg, @Yhg1s, @birkenfeld, @gpshead, @jcea, @amauryfa, @pitrou, @vstinner, @tiran, @asvetlov, @socketpair, @serhiy-storchaka, @1st1, @ajdavis
PRs
  • python/cpython#1715
  • python/cpython#1834
  • python/cpython#1841
  • python/cpython#1843
  • python/cpython#3516
  • python/cpython#3519
  • Files
  • pure-python-atfork.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/gpshead' closed_at = created_at = labels = ['extension-modules', 'interpreter-core', 'type-feature', '3.7'] title = 'Allow registering at-fork handlers' updated_at = user = 'https://github.com/tiran' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'gregory.p.smith' closed = True closed_date = closer = 'gregory.p.smith' components = ['Extension Modules', 'Interpreter Core'] creation = creator = 'christian.heimes' dependencies = [] files = ['28044'] hgrepos = [] issue_num = 16500 keywords = ['patch', 'needs review'] message_count = 58.0 messages = ['175878', '175892', '175967', '175972', '175973', '175974', '175975', '175980', '175997', '176002', '176004', '176019', '176020', '176022', '179838', '179888', '179927', '179945', '179949', '200767', '200774', '200777', '200778', '200779', '200780', '200789', '200797', '200826', '200843', '201892', '201899', '201909', '201912', '266590', '294141', '294145', '294146', '294147', '294148', '294149', '294159', '294597', '294629', '294633', '294634', '294635', '294639', '294640', '294641', '294642', '294651', '294702', '294703', '294705', '294711', '294724', '301983', '302006'] nosy_count = 23.0 nosy_names = ['lemburg', 'twouters', 'georg.brandl', 'gregory.p.smith', 'jcea', 'amaury.forgeotdarc', 'pitrou', 'vstinner', 'christian.heimes', 'grahamd', 'Arfrever', 'ionelmc', 'asvetlov', 'neologix', 'socketpair', 'sbt', 'aliles', 'serhiy.storchaka', 'yselivanov', 'DLitz', 'emptysquare', 'xupeng', 'rpcope1'] pr_nums = ['1715', '1834', '1841', '1843', '3516', '3519'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue16500' versions = ['Python 3.7'] ```

    gpshead commented 7 years ago

    I'll just reuse this issue, I started that work just to try and clean up the new API and docs added in this one to be more modern. Thanks for the code review, good suggestions!

    serhiy-storchaka commented 7 years ago

    The one of potential advantages of the API proposed by Gregory is that os.register_at_fork() can be made atomic. Either register all callbacks, or do nothing in the case of error. But current proposed implementation is not atomic. If resizing of some list is failed due to MemoryError (or may be KeyboardInterrupt), some callbacks can already be registered.

    Is it worth to guarantee the atomicity of os.register_at_fork().

    pitrou commented 7 years ago

    Le 29/05/2017 à 18:35, Serhiy Storchaka a écrit :

    Is it worth to guarantee the atomicity of os.register_at_fork().

    I don't think so, honestly.

    gpshead commented 7 years ago

    New changeset 163468a766e16604bdea04a1ab808c0d3e729e5d by Gregory P. Smith in branch 'master': bpo-16500: Don't use string constants for os.register_at_fork() behavior (bpo-1834) https://github.com/python/cpython/commit/163468a766e16604bdea04a1ab808c0d3e729e5d

    vstinner commented 7 years ago

    New changeset 163468a766e16604bdea04a1ab808c0d3e729e5d by Gregory P. Smith in branch 'master': bpo-16500: Don't use string constants for os.register_at_fork() behavior (bpo-1834) https://github.com/python/cpython/commit/163468a766e16604bdea04a1ab808c0d3e729e5d

    Since there was a disagreement, FYI I also prefer this API. Just the API, I didn't look at the implementation in detail.

    Anyway, it's nice to see this very old issue finally fixed! It was opened 5 years ago!

    I also prefer to have a builtin registered callback in the random module instead of having a custom callback in the multiprocessing module to re-seed the RNG after fork!

    gpshead commented 7 years ago

    This may also allow us to do something reasonable for http://bugs.python.org/issue6721 as well.

    vstinner commented 7 years ago

    New changeset a15d155aadfad232158f530278505cdc6f326f93 by Victor Stinner in branch 'master': bpo-31234: Enhance test_thread.test_forkinthread() (bpo-3516) https://github.com/python/cpython/commit/a15d155aadfad232158f530278505cdc6f326f93

    vstinner commented 7 years ago

    New changeset bcf042ff98b6261b7780c1e40fa1681ef30502f9 by Victor Stinner (Miss Islington (bot)) in branch '3.6': [3.6] bpo-31234: Enhance test_thread.test_forkinthread() (GH-3516) (bpo-3519) https://github.com/python/cpython/commit/bcf042ff98b6261b7780c1e40fa1681ef30502f9