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

Different 3.0a1 exit behavior #45670

Closed
MrJean1 mannequin opened this issue Oct 25, 2007 · 39 comments
Closed

Different 3.0a1 exit behavior #45670

MrJean1 mannequin opened this issue Oct 25, 2007 · 39 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@MrJean1
Copy link
Mannequin

MrJean1 mannequin commented Oct 25, 2007

BPO 1329
Nosy @gvanrossum, @tiran
Files
  • dlibtest.c
  • dlibtest.c
  • stdout-close.patch
  • dlibtest4.c
  • py3k_closefd.patch
  • py3k_combined_preliminary_closefd.patch
  • py3k_preliminary_stderr3.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:

    assignee = 'https://github.com/gvanrossum'
    closed_at = <Date 2007-10-30.17:28:37.210>
    created_at = <Date 2007-10-25.23:07:47.941>
    labels = ['interpreter-core']
    title = 'Different 3.0a1 exit behavior'
    updated_at = <Date 2007-10-30.18:37:53.276>
    user = 'https://bugs.python.org/MrJean1'

    bugs.python.org fields:

    activity = <Date 2007-10-30.18:37:53.276>
    actor = 'gvanrossum'
    assignee = 'gvanrossum'
    closed = True
    closed_date = <Date 2007-10-30.17:28:37.210>
    closer = 'gvanrossum'
    components = ['Interpreter Core']
    creation = <Date 2007-10-25.23:07:47.941>
    creator = 'MrJean1'
    dependencies = []
    files = ['8622', '8625', '8626', '8645', '8647', '8662', '8667']
    hgrepos = []
    issue_num = 1329
    keywords = []
    message_count = 39.0
    messages = ['56761', '56779', '56792', '56793', '56796', '56802', '56810', '56818', '56820', '56821', '56823', '56824', '56825', '56826', '56827', '56831', '56832', '56834', '56835', '56838', '56840', '56844', '56845', '56854', '56856', '56857', '56859', '56882', '56885', '56886', '56890', '56897', '56898', '56944', '56957', '56963', '56964', '56969', '56975']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'nnorwitz', 'christian.heimes', 'MrJean1']
    pr_nums = []
    priority = 'high'
    resolution = 'accepted'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1329'
    versions = ['Python 3.0']

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 25, 2007

    Python 3.0a1 on Linux and MacOS X 10.4.10 exits differently than Python
    2.4 and 2.5.

    With previous Python binaries the destructor** function of any pre-
    loaded shared library is called prior to exit. With Python 3.0a1 it is
    not, neither when exiting Python from the command line with Ctrl-D nor
    when using exit().

    A workaround is to install a SIGABRT signal handler from the library and
    exit Python with os.abort().

    Python 3.0a1 was built from source using the standard build sequence
    without any ./configure options except --prefix.

    ---
    **) defined with GNU __attribute__((destructor)). The shared library is
    loaded through environment variable LD_PRELOAD on Linux and
    DYLD_INSERT_LIBRARIES on MacOS X.

    @MrJean1 MrJean1 mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Oct 25, 2007
    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 26, 2007

    Thanks for testing 3.0.

    Do you have any idea why they are no longer called? I don't recall any
    changes related to this area. Can you try to debug further?

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Here is one thought, maybe 3.0a calls _exit() while 2.x uses exit() to
    terminate. With _exit() any functions installed with atexit() or
    on_exit() are *not* called.

    That would explain the difference but only if destructor functions are
    installed with atexit() or on_exit(). I do not know whether that.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Sorry, premature submit. I will try using atexit() and report what
    happens there.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Using atexit() to install the destructor function does not help. The
    function in not called when 3.0a1 exits, but with 2.5.1 it is. Same
    behavior on Linux and MacOS X.

    Btw, that would mean that any C extension which uses atexit() directly
    may be affected by this issue.

    Running python with the debugger shows that 3.0a1 and 2.5.1 both exit
    thru exit() and not _exit(). A breakpoint at _exit is hit, but the call
    originates from exit and not anywhere in the python binary.

    There is a new atexitmodule.c in 3.0a1 which did not exits in 2.5.1.
    But that is handling the atexit functionality at the Python level and
    not C.

    This man page <http://linux.die.net/man/3/atexit\> mentions that all
    registered atexit functions are removed after a fork+exec. But
    breakpoints set at fork, fork1, forkpty and vfork are never hit by
    3.0a1.

    That is as far as I got.

    @gvanrossum
    Copy link
    Member

    Can you provide a very small shared library that demonstrates this
    problem? (E.g. you could start by modifying Modules/xxmodule.c, adding a
    'destructor' function.)

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Yes, I will make a small library. But first, here is another piece of
    evidence.

    As I mentioned, using std atexit does not work on 3.0a1. But
    surprisingly, the destructor function is not called either when
    installed with Py_AtExit on 3.0a1. On 2.5.1 it is.

    There must something in Py_Terminate or Py_Finalize which is different
    in 3.0a1.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Attached is a simple test case which demonstrates the problem on Linux
    and MacOS X. It is not an xxmodule example, though and hope this is OK.

    See the comments for build steps and example output with 3 different
    Python builds on both platforms.

    If you need another test case which uses Py_AtExit, let me know.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    Here is the same file with an #if to use to Py_AtExit or destructor case.
    Please us this one instead of the earlier one.

    @gvanrossum
    Copy link
    Member

    Here is the same file with an #if to use to Py_AtExit or destructor case.
    Please us this one instead of the earlier one.

    Added file: http://bugs.python.org/file8622/dlibtest.c

    I can build it just fine on Ubuntu dapper, but I can't run it. The
    command given in the comment fails immediately:

    $ env LD_PRELOAD  dlibtest.so  ~/p3/python
    env: LD_PRELOAD: No such file or directory
    $

    When I modify it slightly I get another error:

    $ env LD_PRELOAD=dlibtest.so  ~/p3/python
    ERROR: ld.so: object 'dlibtest.so' from LD_PRELOAD cannot be preloaded: ignored.
    Python 3.0a1+ (py3k, Oct 26 2007, 12:30:11)
    [GCC 4.0.3 (Ubuntu 4.0.3-1ubuntu5)] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> ^D
    $

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    My fault, sorry. The command line to run should be

    env LD_PRELOAD=./dlibtest.so ...

    i.e. with '=' sign and no spaces. And the library file may have to be
    an absolute path.

    @gvanrossum
    Copy link
    Member

    OK, confirmed. But no insignt in what happened yet... Do you know where
    the atexit stuff happens in 2.5?

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    The Py_AtExit function is in Python/pythonrun.c. The calls to all
    installed C functions are made in call_ll_exitfuncs, also in
    pythonrun.c. The call to call_ll_exitfuncs is at the very end of
    Py_Finalize also in pythonrun.c.

    I am just getting down there now and Py_Finalize is called and reaches
    the call to PyGrammar_RemoveAccelerators a few lines higher. But
    call_ll_exitfuncs is not called, as far as I can tell.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    One more thing. Stepping with the debugger thru Py_Finalize looks exactly
    the same for 2.5.1 and 3.0a1 in the last part of that function.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    I put a bunch of printf's in the Py_Finalize function of 2.5.1 and
    3.0a1.

    All those show up when 2.5.1 exists including the call to
    call_ll_exitfuncs.

    But in 3.0a1 only a few show up and the last one is just before the call
    to PyImport_Cleanup near line 393. It looks like that call never
    returns. That call never returns, it seems.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 26, 2007

    Maybe that's because site and io get cleaned up and then there is some
    fatal error that can't be printed?

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 26, 2007

    This is quite bizarre and difficult to reproduce. With gdb, 3.0a1 does
    get to the very end of Py_Finalize, but without gdb it doesn't.

    Also, the call to static function call_all_exitfuncs is inlined, its
    while loop is shown inside Py_Finalize on MacOS X. On Linux that is not
    visible, probably due to differences in gcc/gdb, 4.0.1 or MacOS X and
    3.4.6 on Linux.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 27, 2007

    I suggest you configure --with-pydebug. That will disable
    optimization, enable debugging symbols and will make it easier to
    develop. Note that a debug version runs much slower due to asserts()
    and other internal changes. Otherwise, it should work the same.

    On Oct 26, 2007 4:55 PM, Jean Brouwers <report@bugs.python.org> wrote:

    Jean Brouwers added the comment:

    This is quite bizarre and difficult to reproduce. With gdb, 3.0a1 does
    get to the very end of Py_Finalize, but without gdb it doesn't.

    Also, the call to static function call_all_exitfuncs is inlined, its
    while loop is shown inside Py_Finalize on MacOS X. On Linux that is not
    visible, probably due to differences in gcc/gdb, 4.0.1 or MacOS X and
    3.4.6 on Linux.


    Tracker <report@bugs.python.org>
    <http://bugs.python.org/issue1329\>


    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    OK, I try that.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    The 3.0a1 build --with-pydebug behaves the same as before (on Linux). The
    problem does occur without gdb but not with gdb.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    It looks like the problem may indeed just be that I/O is being shut down
    somewhere inside PyImport_Cleanup. I am modifying the test case to
    demonstrate that and will submit that version as soon as it runs.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    Attached is an updated dlibtest.c file. It prints a message in the con-
    /destructor functions and if that fails it calls _exit(9).

    Compile and run it as before and check the exit status. If the latter
    is 9 or 011, a printf error occurred indicating e.g. that stdout was
    closed or something similar.

    This version can also be used with gdb, either by pre-loading the
    dlibtest.so library within gdb or before invoking gdb. To preload the
    library within gdb (on Linux) use

    gdb .../python
    (gdb) set environment LD_PRELOAD ./dlibtest.so
    (gdb) run
    .....

    or to preload before gdb use

    setenv LD_PRELOAD ./dlibtest.so
    gdb .../python
    (gdb) run
    .....

    Lastly, my previous observations about this issue were clearly a "trompe
    d'oeil", especially my statement that PyImport_Cleanup never returned.
    The missing print statements *after* the PyImport_Cleanup call are
    simply due to printf errors, and nothing else ;-)

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Oct 27, 2007

    When I run with the attached patch, I see the message:

    *** dtor called in python ...

    Is that the behavior you expect?

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    Yes, that is the expected behavior in this case.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    One final comment as confirmation. If the messages are written to a file,
    other than stdout and stderr, they do appear in unpatched 3.0a1 and 3.0a1
    behaves as expected. The root cause of the problem was the closed stdout.

    @gvanrossum
    Copy link
    Member

    So is there even a bug? Arguably you shouldn't be writing anything that
    late in the life of a shared library.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 27, 2007

    It is quite common to pre-load libraries into existing binaries e.g. for
    profiling Typically, those write to stdout or stderr only if the option
    for an output file is not used. That is how I happened to run into the
    issue the other day.

    For a while, the initial symptoms looked like the different exit
    behavior in 3.0a1 might be a serious problem. It was not obvious that
    stdout and -err might have been closed and caused the difference. All
    the Python 2.x versions never closed stdout and -err.

    Therefore, 3.0 should probably not do that either. But that is really
    your call.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 28, 2007

    One more argument. Without a fix, 3.0 would not even print a C debug
    message from a destructor function nor from any function installed with
    atexit or Py_AtExit. The dlibtest shows that for 2 of these 3.

    @tiran
    Copy link
    Member

    tiran commented Oct 28, 2007

    Can you try this patch, please? It has the same effect as the other
    patch from Neal but it doesn't loose ref counts. I've patched the
    dealloc function of _FileIO to keep fd 1 and fd 2 open.

    Index: Modules/_fileio.c
    ===================================================================

    --- Modules/_fileio.c   (Revision 58699)
    +++ Modules/_fileio.c   (Arbeitskopie)
    @@ -270,7 +270,8 @@
            if (self->weakreflist != NULL)
                    PyObject_ClearWeakRefs((PyObject *) self);
    -       if (self->fd >= 0) {
    +       /* Don't close stdout and stderr */
    +       if (self->fd == 0 || self->fd > 2) {
                    errno = internal_close(self);
                    if (errno < 0) {
     #ifdef HAVE_STRERROR

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 28, 2007

    I could not try Neal's patch since it does not seem to apply to the
    3.0a1 source I have. But the Modules/_fileio.c patch works just fine on
    my Linux and MacOS X. Here is the Linux result:

    $ env  LD_PRELOAD=./dlibtest4.so  ~/Python-3dbg/python
    *** ctor called in python ...
    *** atexit OK in python ...
    Python 3.0a1 (py3k, Oct 28 2007, 10:23:59) 
    [GCC 3.4.6 20060404 (Red Hat 3.4.6-8)] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> 
    [36000 refs]
    [21985 refs]
    *** dtor called in python ...
    $

    Most interesting is that this Python build --with-pydebug now prints 2
    lines in [..] brackets on exit. That 2nd line, [21985 refs] never
    showed up before!

    Also, attached is another version of my test case renamed to dlibtest4.
    It includes 4 different use cases. More details inside.

    @MrJean1
    Copy link
    Mannequin Author

    MrJean1 mannequin commented Oct 28, 2007

    Perhaps, the proper behavior is the following.

    After calling all functions/methods installed at the Python level with
    atexit.register and all C functions installed with Py_AtExit, the
    objects sys.stdin, sys.stdout and sys.stderr are destroyed.

    However, the C level files stdin, stdout and stderr are *never* closed
    by Python. Those will be closed (like any other open file) after all
    functions installed with atexit or defined as destructors have been
    called.

    @gvanrossum
    Copy link
    Member

    Right. I think the right solution is to add an option to _FileIO that
    says "don't close the filedescriptor when close() is called". This
    option should only be allowed when the "filename" argument is an
    integer file descriptor. It should be passed when stdin/out/err are
    created. It may also be helpful in some other places?!

    @tiran
    Copy link
    Member

    tiran commented Oct 29, 2007

    Here you go, Guido!

    @gvanrossum gvanrossum self-assigned this Oct 29, 2007
    @gvanrossum
    Copy link
    Member

    Thanks!! Code review:

    Shouldn't closefd be passed as 1 in import.c?

    I don't see the point of distinguishing between -1 and +1. The block
    "if (closefd < 0) { closefd = 1; }" looks rather silly.

    In io.py, you should document that closefd must not be False when a
    filename is given.

    I think in _fileio.c, you can insist that the closefd argument is an int
    (a bool will work anyway, as bool is a subclass of int).

    I don't think we should warn when trying to close an unclosable fd; it
    should really just be a no-op. Also, if you are going to call
    PyErr_WarnEx(), you should test its return value (it can raise an
    exception!).

    Please don't add trailing whitespace.

    @tiran
    Copy link
    Member

    tiran commented Oct 30, 2007

    Guido van Rossum wrote:

    Shouldn't closefd be passed as 1 in import.c?

    I don't see the point of distinguishing between -1 and +1. The block
    "if (closefd < 0) { closefd = 1; }" looks rather silly.

    I used -1 as default to keep it consistent with buffer=-1. I figured out
    that I can go with "closefd != 0 means close it".

    In io.py, you should document that closefd must not be False when a
    filename is given.

    Done

    I think in _fileio.c, you can insist that the closefd argument is an int
    (a bool will work anyway, as bool is a subclass of int).

    Thanks, it makes the code a bit easier.

    I don't think we should warn when trying to close an unclosable fd; it
    should really just be a no-op. Also, if you are going to call
    PyErr_WarnEx(), you should test its return value (it can raise an
    exception!).

    I think we should keep the warning. The warning made me aware of a minor
    bug in quopri.

    Please don't add trailing whitespace.

    I've reconfigured my editor to remove trailing spaces.

    I've attached a combined patch for closefd and preliminary stderr.

    Christian

    @gvanrossum
    Copy link
    Member

    OK, thanks. The closefd part is good, but the stderrprinter part has a
    problem. On Linux, in a non-debug build, this has the odd side effect
    of subtracting one from sys.maxunicode. In a debug build, it dies like
    this:

    $ ./python -S
    python: Modules/gcmodule.c:336: visit_reachable: Assertion `gc_refs > 0
    || gc_refs == (-3) || gc_refs == (-2)' failed.
    Aborted
    $ 

    If I comment out the PySys_SetObject() call everything seems fine, but I
    suspect that the problem is actually in the creation of the stdprinter
    object.

    @gvanrossum
    Copy link
    Member

    I've checked the closefd patch (which minor changes) into the py3k branch.

    Committed revision 58711.

    Please take the stdprinter patch to the original issue (bug 1352).

    @tiran
    Copy link
    Member

    tiran commented Oct 30, 2007

    Guido van Rossum wrote:

    Guido van Rossum added the comment:

    OK, thanks. The closefd part is good, but the stderrprinter part has a
    problem. On Linux, in a non-debug build, this has the odd side effect
    of subtracting one from sys.maxunicode. In a debug build, it dies like
    this:

    $ ./python -S
    python: Modules/gcmodule.c:336: visit_reachable: Assertion `gc_refs > 0
    || gc_refs == (-3) || gc_refs == (-2)' failed.
    Aborted
    $

    If I comment out the PySys_SetObject() call everything seems fine, but I
    suspect that the problem is actually in the creation of the stdprinter
    object.

    I may have found the problem. I forgot th remove Py_TPFLAGS_HAVE_GC from
    tp_flags. It's a relict from my first implementation.

    $ ./python
    Fatal Python error: Py_Initialize: can't initialize sys standard streams
    Traceback (most recent call last):
      File "/home/heimes/dev/python/py3k/Lib/io.py", line 22, in <module>
        test
    NameError: name 'test' is not defined
    Aborted
    $ vi Lib/io.py
    $ ./python -S
    Python 3.0a1+ (py3k:58715M, Oct 30 2007, 19:02:47)
    [GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
    >>> import sys
    [33116 refs]
    >>> sys.maxunicode
    1114111
    [33127 refs]
    >>>
    [33128 refs]
    [23233 refs]
    $ python2.5 -c "import sys; print sys.maxunicode"
    1114111

    @gvanrossum
    Copy link
    Member

    Thanks, I've closed bpo-1352 too now.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants