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.

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

[doc] sys.exit() called from threads other than the main one: undocumented behaviour #50883

Closed
jgehrcke mannequin opened this issue Aug 3, 2009 · 27 comments
Closed

[doc] sys.exit() called from threads other than the main one: undocumented behaviour #50883

jgehrcke mannequin opened this issue Aug 3, 2009 · 27 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jgehrcke
Copy link
Mannequin

jgehrcke mannequin commented Aug 3, 2009

BPO 6634
Nosy @terryjreedy, @pitrou, @jgehrcke, @vadmium, @miss-islington, @iritkatriel, @Vidhyavinu
PRs
  • bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter #31639
  • [3.10] bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter (GH-31639) #31660
  • [3.9] bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter (GH-31639) #31661
  • Files
  • issue6634_py35.patch: threading.py and test_threading.py patch (against tip).
  • issue6634_py27.patch: Docs patch for head in 2.7 branch.
  • issue6634_py27.patch: Docs patch for head in 2.7 branch.
  • 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:

    assignee = None
    closed_at = <Date 2022-03-03.15:17:31.054>
    created_at = <Date 2009-08-03.19:21:49.940>
    labels = ['easy', 'type-bug', '3.9', '3.10', '3.11', 'library', 'docs']
    title = '[doc] sys.exit() called from threads other than the main one: undocumented behaviour'
    updated_at = <Date 2022-03-03.15:18:42.408>
    user = 'https://github.com/jgehrcke'

    bugs.python.org fields:

    activity = <Date 2022-03-03.15:18:42.408>
    actor = 'vidhya'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2022-03-03.15:17:31.054>
    closer = 'iritkatriel'
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2009-08-03.19:21:49.940>
    creator = 'jgehrcke'
    dependencies = []
    files = ['37989', '37990', '38096']
    hgrepos = []
    issue_num = 6634
    keywords = ['patch', 'easy']
    message_count = 27.0
    messages = ['91237', '91288', '112692', '117363', '117378', '156673', '156691', '156838', '235280', '235287', '235288', '235296', '235312', '235324', '235344', '235635', '235652', '235740', '408374', '414229', '414284', '414298', '414443', '414445', '414447', '414448', '414449']
    nosy_count = 12.0
    nosy_names = ['terry.reedy', 'ggenellina', 'pitrou', 'stutzbach', 'Arfrever', 'jgehrcke', 'docs@python', 'martin.panter', 'David.Manowitz', 'miss-islington', 'iritkatriel', 'vidhya']
    pr_nums = ['31639', '31660', '31661']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue6634'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Aug 3, 2009

    Hey there,

    hopefully I fill out this form in an adequate way!

    I ran into some problems while using sys.exit('msg') together with
    threads, which could have been avoided with slightly more information in
    the docs here: http://docs.python.org/library/sys.html#sys.exit

    Maybe the following two statements should not stay as they are:

    (1) "Exit from Python."
    -----------------------
    This is not true when called from a thread other than the main one. We
    could add a hint, saying that sys.exit() then actually behaves like
    thread.exit(), which causes only the calling thread to exit, but not the
    main program.

    1. "[...] and any other object is printed to sys.stderr"
      --------------------------------------------------------
      This is also not true when called from a thread other than the main one.
      Calling sys.exit('msg') then doesn't print anything to stderr. That was
      annoying in my case and required debugging a bug that would have
      discovered itself via stderr, if the message would have been printed..
      :-) After some research, I think this behaviour is described in the
      documentation for thread.exit(): "[...] this will cause the thread to
      exit *silently*."

    Okay, now that I am aware of this behaviour, I won't run into these
    problems again. But the next one?

    I think (1) is clearly a documentation thing. Regarding (2): first of
    all, the documentation should say that the message is suppressed in
    special cases (child threads). But: what argues against printing to
    stderr here? I don't get the point and only see a lost feature,
    affording a quick way to kill a thread while dropping an error message.
    Was this kicked out intentionally? Maybe someone could help me with a
    good argument here :-)

    Thank you for your work,

    Jan-Philip Gehrcke

    @jgehrcke jgehrcke mannequin assigned birkenfeld Aug 3, 2009
    @jgehrcke jgehrcke mannequin added the docs Documentation in the Doc dir label Aug 3, 2009
    @ggenellina
    Copy link
    Mannequin

    ggenellina mannequin commented Aug 5, 2009

    I agree with you; the docs should be improved, and I see no reason for
    sys.exit("msg") NOT to write to stderr inside a child thread.

    @terryjreedy
    Copy link
    Member

    Please suggest a specific doc change at a specific location.
    IE, how *should* they read, not just what is wrong.

    @terryjreedy terryjreedy assigned docspython and unassigned birkenfeld Aug 3, 2010
    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Sep 25, 2010

    Sorry for the delay.

    Before suggesting a doc change to correct/complete the description of the *current* situation, we actually should consider changing this situation. I think this is reasonable and I feel encouraged by Gabriel Genellina:

    I see no reason for sys.exit("msg") NOT to write to stderr
    inside a child thread.

    This patch enables printing to stderr from child threads and clones the behavior of sys.exit(arg) called from the main thread:

    # PATCH BEGIN
    --- C:/Python27/Lib/threading.py    Sat Apr 10 18:55:48 2010
    +++ C:/python_sys_exit_issue/threading.py    Sat Sep 25 14:50:24 2010
    @@ -531,6 +531,15 @@
     except SystemExit:
         if __debug__:
             self._note("%s.__bootstrap(): raised SystemExit", self)
    +    # Now get and handle the "exit code", given by the user via
    +    # the second expression after `raise` or via the argument of 
    +    # sys.exit().
    +    code = self.__exc_info()[1].code
    +    # Ignore None and integer exit code. Print any other object
    +    # to stderr as it is the behavior of sys.exit(arg) called
    +    # from the main thread.
    +    if code is not None and not isinstance(code, int):
    +        _sys.stderr.write("%s\n" % code)
     except:
         if __debug__:
             self._note("%s.__bootstrap(): unhandled exception", self)
    # PATCH END

    A script with different testcases including output is attached.

    What do you think?

    All the best,

    Jan-Philip Gehrcke

    @terryjreedy
    Copy link
    Member

    I do not use threads so I cannot comment on the technical issue.

    Since the current behavior is not clearly a bug, I do not think a change would or know that it should be applied to 2.7/3.1. So I suggest that you both

    1. Suggest a doc patch on this issue. That should not controversial and might be quickly applied.

    2. Submit a separate feature request issue to change behavior in 3.2 or beyond. You could request that the change be backported to 2.7 and try to make a case for doing so.

    Unless you can quickly write a tested patch, such a change in unlikely to make it into 3.2 anyway. And even then, no guarantee.

    @DavidManowitz
    Copy link
    Mannequin

    DavidManowitz mannequin commented Mar 23, 2012

    I don't see why this should be considered acceptable behavior. Why don't threads have their own ThreadExit exception, rather than overloading the use, and therefore, the meaning, of the SystemExit exception? As indicated by their names, sys.exit and the SystemExit exception should *only* be used to exit the entire system, not just a thread!

    @DavidManowitz DavidManowitz mannequin added the stdlib Python modules in the Lib dir label Mar 23, 2012
    @pitrou
    Copy link
    Member

    pitrou commented Mar 24, 2012

    I don't see why this should be considered acceptable behavior. Why
    don't threads have their own ThreadExit exception, rather than
    overloading the use, and therefore, the meaning, of the SystemExit
    exception? As indicated by their names, sys.exit and the SystemExit
    exception should *only* be used to exit the entire system, not just a
    thread!

    I agree the situation isn't optimal. However, fixing this would also break compatibility with any application that uses sys.exit() in a thread and expects it to exit the thread, not the whole process. So we're kind of stuck with it.

    @DavidManowitz
    Copy link
    Mannequin

    DavidManowitz mannequin commented Mar 26, 2012

    I have a couple of issues with that argument:

    1.) Until fairly recently, the fact that sys.exit() when called from a
    non-primary thread only causes the thread to die, was not clearly
    documented (and still isn't in the python2.6 docs). Admittedly,
    thread.exit() does say that it raises the SystemExit exception, but as most
    people are encouraged to use the *threading* module, rather than the
    *thread* module directly, this is still fairly obscure.

    2.) A ThreadExit exception could be derived from the SystemExit exception,
    so existing code that works by catching a SystemExit exception would still
    work.

    --David

    On Sat, Mar 24, 2012 at 6:47 AM, Antoine Pitrou <report@bugs.python.org>wrote:

    Antoine Pitrou <pitrou@free.fr> added the comment:

    > I don't see why this should be considered acceptable behavior. Why
    > don't threads have their own ThreadExit exception, rather than
    > overloading the use, and therefore, the meaning, of the SystemExit
    > exception? As indicated by their names, sys.exit and the SystemExit
    > exception should *only* be used to exit the entire system, not just a
    > thread!

    I agree the situation isn't optimal. However, fixing this would also break
    compatibility with any application that uses sys.exit() in a thread and
    expects it to exit the thread, not the whole process. So we're kind of
    stuck with it.

    ----------
    nosy: +pitrou


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue6634\>


    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 2, 2015

    For Python 2.7, we will not change behavior, even if unexpected. Instead, the sys.exit-docs should be adjusted and

    - warn about the fact that nothing is written to stderr
      if sys.exit(msg) gets called from a non-primary thread, and
    
    - note that SystemExit raised in a non-primary thread lets the
      thread exit silently.
    

    I have attached a corresponding patch, please review my wording!

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 2, 2015

    For Python 3.5, I have attached a patch that

    - adds relevant test cases to test_threading.py which probe
      the interpreter's stderr output for compliance with what
      the docs state.
    
    - makes sys.exit(msg) write msg to stderr, even if called
      from a non-primary thread, so that the tests succeed.
    

    If we take this path, the documentation for 3.5 does not need to be adjusted.

    The discussion in this thread diversified itself a bit:

    Why don't threads have their own ThreadExit exception,
    rather than overloading the use, and therefore, the
    meaning, of the SystemExit exception? sys.exit and the
    SystemExit exception should *only* be used to exit the
    entire system, not just a thread!

    While I absolutely agree that this would be conceptually cleaner, implementing this would be a larger refactoring task. Deciding whether this should be done or not slows this issue down, and I think this discussion should probably be taken elsewhere.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 2, 2015

    Regarding the documentation patch: I like to start sentences with a capital letter. Perhaps change it to start “Calling :func:`exit` only terminates . . .”.

    With the code change patch, it might be neater to use the SystemExit.code attribute rather than e.args[0].

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 2, 2015

    Regarding the documentation patch: I like to start sentences
    with a capital letter. Perhaps change it to start
    “Calling :func:`exit` only terminates . . .”.

    Thanks for feedback. Have now used "Invocation of ...." to not repeat "call*" in the sentence, because I left the "when called from the main thread" part unchanged. Okay?

    With the code change patch, it might be neater to use
    the SystemExit.code attribute rather than e.args[0].

    Oh, thanks. Was not aware of the existence of the code attribute. If anyone else was wondering: existence and behavior are defined in Objects/exceptions.c via static PyMemberDef SystemExit_members[] and via static int SystemExit_init().

    It is populated upon construction of a SystemExit instance:

    Py_CLEAR(self->code);
    if (size == 1)
        self->code = PyTuple_GET_ITEM(args, 0);
    else /* size > 1 */
        self->code = args;
        
    Hence, the translation from arguments to exit code considers *all* arguments. I adjusted the patch to use the `code` attribute.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 3, 2015

    New patches look fine. BTW SystemExit.code is also documented at <https://docs.python.org/dev/library/exceptions.html#SystemExit\>.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 3, 2015

    I'm not sure what the doc patch achieves. It only states more verbosely what is already said in the current version.
    Also, warnings are really for important issues (such as security issues); using them too liberally is a disservice to the reader.

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 3, 2015

    Thanks for your feedback Antoine.

    I'm not sure what the doc patch achieves.

    Let me try to bring things in order. It should achieve two things:

    1. Properly describe the stderr-writing behavior of sys.exit().
      ===============================================================
      Current 2.7 docs:

      "..., and any other object is printed to stderr".

    This is wrong in its generality and requires clarification. Agreed? How would you improve the docs in this regard? This is the reasoning I had in mind:

    We could change the original sentence, but IMO it then contains too many pieces of important information and becomes difficult to digest, e.g.: "If another type of object is passed, None is equivalent to passing zero, and any other object is printed to stderr (only when called in the main thread) and results in an exit code of 1.". Difficult, right?

    I thought about removing the stderr part from this sentence and discussing this topic separately, in simpler sentences. But: I think the stderr part needs to stay in the original sentence, because it clarifies how different argument types are dealt with.

    Then I took another point of view: we have acknowledged that the behavior is problematic, we just cannot change it anymore for 2.7. The direct consequence from this point of view is to warn about current behavior. 6 years ago, I took the docs literally and relied on getting proper error messages printed to stderr. And I didn't get those. I would have appreciated a warning, I guess.

    With this being said, I'd love to look at an alternative proposal of how we could change the docs in this regard.

    1. Clarify that when called from a thread, the thread exits silently
      ====================================================================
      Current 2.7 docs:

      "Since exit ultimately "only" raises an exception,
      it will only exit the process when called from the main thread."

    This sentence is problematic. It relates a cause to an effect, and this relation is wrong or at least incomplete. The only valuable information left in the sentence is the *effect*, without providing an explanation. I would rather want to take the inverse approach, and explain the *cause* as correct as possible. In other words: the above sentence misses to explain what actually happens when sys.exit() is called in a non-primary thread.

    The actual behavior and cause for mentioned effect is, as far as I am aware:

    If raised in a non-primary thread, SystemExit is caught automatically and causes the calling thread to exit silently.

    This is what I added as a note.

    And I think that we agree that this cause may have many effects, whereas only one of them is that sys.exit() called from a non-primary thread can not terminate the process.

    It only states more verbosely what is already
    said in the current version.

    I hope to have convinced you that this is not true.

    Also, warnings are really for important issues
    (such as security issues); using them too liberally
    is a disservice to the reader.

    I agree. I really think that both points need to be addressed in the docs, and I have now clarified my reasoning, but am very open to further suggestions. And I really do not want to make a strong point about whether things should end up in red or blue boxes :-).

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 9, 2015

    I'd love to find an agreement here. I think we are quite close to getting this closed, so further input is very welcome.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 10, 2015

    If it were me, I’d drop the Notes and Warnings (never been a fan), and put Thing #2 first, and then Thing #1. Maybe something like

    '''
    sys.exit([arg])

    Exit from Python. . . . and it is possible to intercept the exit attempt at an outer level. When called from a thread other than the main thread, this causes the thread to exit silently instead, and is equivalent to calling :func:`thread.exit`.

    The optional argument arg can be an integer giving the exit status (defaulting to zero). Passing None is equivalent to passing zero. Any other object is printed to stderr and results in an exit status of 1. In
    particular, sys.exit("some error message") is a quick way to exit a
    program when an error occurs. When called from a thread other than the main thread, nothing is printed and the argument is ignored.

    An exit status of zero is considered “successful termination” and any nonzero status is considered “abnormal termination” . . . Unix programs generally use 2 for command line syntax errors and 1 for all other kinds of errors.
    '''

    @jgehrcke
    Copy link
    Mannequin Author

    jgehrcke mannequin commented Feb 11, 2015

    Martin, I very much like the order you suggested, thanks. I did not feel confident enough for re-structuring the entire entry. So, can we agree on using that for Python 2.7?

    Is there a consensus regarding the approach to take for Python 3.5? Except from Martin there was no feedback on the patch.

    @iritkatriel
    Copy link
    Member

    Since 3.8 we have threading.excepthook() so the situation is different now:

    #13515

    I think it still makes sense to change the wording of the doc for sys.exit() a bit, and add a reference to the relevant section of the threading doc.

    @iritkatriel iritkatriel added easy 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes labels Dec 12, 2021
    @iritkatriel iritkatriel changed the title sys.exit() called from threads other than the main one: undocumented behaviour [doc] sys.exit() called from threads other than the main one: undocumented behaviour Dec 12, 2021
    @iritkatriel iritkatriel added the type-bug An unexpected behavior, bug, or error label Dec 12, 2021
    @Vidhyavinu
    Copy link
    Mannequin

    Vidhyavinu mannequin commented Mar 1, 2022

    [Entry level contributor seeking guidance]
    If this is still open, I can work on this.

    I plan to add the following in sys.exit() and add a reference to thread.exit() at https://docs.python.org/3/library/sys.html#sys.exit:

    When called from a thread other than the main thread, this causes the thread to exit silently, and is equivalent to calling :func:`thread.exit`.

    Please correct if anything wrong.

    @iritkatriel
    Copy link
    Member

    Vidhya, I think the sentence you are suggesting to add would overlap with one which is already there ("Since exit() ultimately “only” raises an exception, it will only exit the process when called from the main thread, and the exception is not intercepted.")

    It seems to me that what could be improved is the first paragraph, which starts with: "Exit from Python.". Maybe instead it could say something like "Raise a SystemExit exception, which has the effect of ..." and then say what it is (i.e., that it exits python if you are in the main thread, unless the exception is caught... ).

    @Vidhyavinu
    Copy link
    Mannequin

    Vidhyavinu mannequin commented Mar 1, 2022

    Thanks for your comments :). The PR for the same is:
    #31639

    @iritkatriel
    Copy link
    Member

    New changeset 10117f1 by vidhya in branch 'main':
    bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter (GH-31639)
    10117f1

    @miss-islington
    Copy link
    Contributor

    New changeset 9d9dc59 by Miss Islington (bot) in branch '3.10':
    bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter (GH-31639)
    9d9dc59

    @iritkatriel
    Copy link
    Member

    New changeset 0981986 by Miss Islington (bot) in branch '3.9':
    bpo-6634: [doc] clarify that sys.exit() does not always exit the interpreter (GH-31639) (GH-31661)
    0981986

    @iritkatriel
    Copy link
    Member

    Thank you @Vidhya.

    @Vidhyavinu
    Copy link
    Mannequin

    Vidhyavinu mannequin commented Mar 3, 2022

    Thanks Irit for your help.

    On Thu., Mar. 3, 2022, 10:17 a.m. Irit Katriel, <report@bugs.python.org>
    wrote:

    Irit Katriel <iritkatriel@gmail.com> added the comment:

    Thank you @Vidhya.

    ----------
    resolution: -> fixed
    stage: patch review -> resolved
    status: open -> closed


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue6634\>


    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants