#461 Allow inherited error mode.

closed-accepted
Cesar Strauss
msys (22)
2011-09-18
2010-10-05
Peter Rosin
No

Hi!

I am a libtool developer and I have grown *real* tired of the popups displayed
when the libtool testsuite is running. Libtool tries various things that is
supposed to not work, such as running programs with missing dlls. That
of course triggers a tiresome popup which you have to dismiss. Every time.
So, I thought that it would be neat if you could set the error mode to something
that is not the default, and have that error mode propagate to all child
processes. This patch is what I came up with. I know that I'm touching parts
of the code that are considered scary, and I'm a bit of a newbie msys/cygwin
hacker so there might easily be better ways to do this. But it works for me,
and I don't think it's going to affect the normal case.

The only thing I can think of is a system with a defalt error mode that
includes the SEM_FAILCRITICALERRORS flag, but the only thing that will
happen on those systems is that whatever error mode happes to be
active when entering msys is going to be inherited to all children. But it
would be a rare case indeed if that would not be the default error mode.
That, and msys is going to be running with that error mode and not with
only SEM_FAILCRITICALERRORS (since msys drops all (non-sticky) flags
but that one without this patch).

In order to actually trigger this patch, I'm using the error-mode.c
program, which I intend to attach next.

Cheers,
Peter

Discussion

  • Peter Rosin
    Peter Rosin
    2010-10-05

    [PATCH] Allow the error mode to be overridden.

     
    Attachments
  • Peter Rosin
    Peter Rosin
    2010-10-05

    Oh, I forgot to mention that this error-mode program must be compiled
    as a MinGW program, and not an MSYS program. If it's an MSYS program,
    the exec call will restore the default error mode before the new process
    is started.

    Here's the help text:
    Usage: error-mode [options] [program [program options]]
    -c, --no-critical-fail No critical failure error box
    -g, --no-general-protection No general fault error box
    -f, --no-file-open No open file error box
    -m MODE, --mode=MODE Desired SetErrorMode argument
    -h, --help Print this message and exit
    -v, --version Print the version number and exit

    Example:
    error-mode -cgf C:/mingw/msys/1.0/msys.bat

     
  • Peter Rosin
    Peter Rosin
    2010-10-05

    error-mode version 0.2

     
    Attachments
  • Cesar Strauss
    Cesar Strauss
    2010-10-20

    Dear Peter,

    Thanks for the patch, it looks interesting.

    I'll try to find some time to test it properly.

    Thanks,
    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2010-10-20

    • assigned_to: nobody --> cstrauss
     
  • Peter Rosin
    Peter Rosin
    2010-10-20

    Hi Cesar,

    Godd to hear, my fingers was itching with the desire to ping this patch, but now I
    know it has been seen which makes me relax. For a while :-)

    FWIW, I've been running with it since I created it with no ill effects that I can spot.
    But I stopped trying to stress it when I was satisfied with it so that is perhaps not
    definitive. And I don't expect you to take my word for it either...

    Cheers,
    Peter

     
  • Charles Wilson
    Charles Wilson
    2010-10-25

    FWIW, I ran into the old "fork: blah blah" problem with MSYS-1.0.16 on my w64 box. However...I found two ways to fix it:

    #1) Use C:\Windows\SysWOW64\cmd.exe instead of 'cmd.exe', and launch the bat file "by hand", or...

    #2) Use the error_mode program to launch the bat file (this doesn't require a patched msys dll; you don't even need to specify any of error_mode's arguments). The reason this works is (I think) because error_mode launches the target using _execvp, which under the hood runs CreateProcess in such a way that the stack is in the "good location". This old post:
    http://osdir.com/ml/gnu.mingw.msys/2004-03/msg00035.html
    suggests that on w64, the memory layout of an app launched directly by the windows system (using Nt* or z* functions rather than normal w32api ones?) *differs* from that of an app launched via CreateProcess (???). So, by ensuring that our "top level" msys app was also launched via create process (indirectly, by way of execvp), our eventual parent process will have the same memory layout as any of its children.

    I bring this up, because once Peter's patch is merged (and error_mode.exe is shipping in msys-1.0.nn), we'll have another thing to suggest that people try, if they run into fork blah blah errors with msys on w64.

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-20

    Always inherit the error mode flag.

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-20

    Sorry for taking so long.

    Before going forward with Peter's patch, I would like to suggest another approach.

    How about we simply stop forcing the error mode and always inherit the error mode for child processes? In other words, drop the existing SetErrorMode call and the CREATE_DEFAULT_ERROR_MODE flag for CreateProcess (see attached patch). I tried it and could not see any difference in MSYS behavior arising from the lack of the SEM_FAILCRITICALERRORS error mode at runtime.

    Regards,
    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-20

    I also attach a proposed new version of the error-mode program with these changes:

    1) Use a synchronous spawnvp instead of execvp. Otherwise, the parent and the child compete for the console.

    2) When calling a process with arguments, it is best to precede the command line with "--", otherwise the program arguments would be interpreted as options for error-mode.exe. I updated the help message accordingly.

    Regards,
    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-20

    error-mode version 0.3

     
    Attachments
  • Peter Rosin
    Peter Rosin
    2010-11-21

    Simply removing SetErrorMode was my first thought as well. But you then get a *big* change in semantics. All the code inside the msys dll will now (in the normal case at least) run with the risk of throwing popups. E.g. if msys tries to LoadLibrary a DLL that for some reason lacks a dependency, a previously graceful fail might not be so graceful anylonger. I don't know if there is any code in msys that is better off with graceful failures, and I don't expect to be able to prove that there isn't, so you are pretty much on your own if you want to try your proposed patch. In other words, don't blame me :-)

    However, if you do go with your much simpler patch, then the need for the error-mode program is diminished, as the method used to start the inferior is no longer crucial. It'd be much easier to add the -cgf options (I really like that homage, btw) to one of the preexisting wrappers.

    BTW, first line of your error-mode.c should now read "sets the error mode and spawns".

    I don't understand what you are referring to with your "--" remark? Was that just a reminder for the future that whatever/whomever invokes the error-mode program should add that? Because I think error-mode supports "--" as written, right? Are you referring to the example in the help output?

    Cheers,
    Peter

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-25

    > Simply removing SetErrorMode was my first thought as well.
    > But you then get a *big* change in semantics.

    Agreed. I'll let the other MSYS developers give an opinion before going ahead with this.

    Meanwhile, I managed to simplify your patch a bit. I used the state of error flag at DLL entry as the sole means of communication between parent and child.

    The idea is to always check the error mode at DLL entry. If the SEM_FAILCRITICALERRORS flag was set, make the error mode inheritable, otherwise request the default error mode.

    Let me know if the attached patch woks for you.

    > I don't understand what you are referring to with your "--" remark?

    I am referring to the usage information in the help output. For instance, the following works:

    error_mode -cgf -- bash --login -i

    But without the "--", it doesn't. So I wanted to make it clear in the help text.

    > the -cgf options (I really like that homage, btw)

    I was wondering if it was a coincidence ... :-)

    Regards,
    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2010-11-25

    Always check the error mode at startup

     
    Attachments
  • Charles Wilson
    Charles Wilson
    2011-04-03

    error-mode.c v0.3 + error-mode-v2.patch, for direct inclusion in CVS

     
  • Charles Wilson
    Charles Wilson
    2011-04-03

    I've uploaded a combined version of Cesar's most recent patchset. Using a DLL built with that patch, and with the error-mode executable installed, I did the following mega stress tests:

    1) launching an msys-dvlpr shell in "normal" mode, ran the entire five hour libtool test suite. No regressions over the 2.4-1-msys-1.0.15 release, AND during the test, as expected, I got a number of popup windows. This is the normal behavior on msys.

    2) closing everything, and re-launching an msys-dvlpr shell via error-mode -cgf, I re-ran the entire libtool test suite. Again, no regressions over 2.4-1-msys-1.0.15. However, during the test I got NO popup windows, and the test ran to completion without user interaction (Yay!!!)

    I think this should be added to MSYS cvs and included in the next release of msys (soon?)

    FYI, I ran the tests with the new perl-5.8.8 installed.

     
  • Cesar Strauss
    Cesar Strauss
    2011-04-05

    Thanks, Chuck, for testing the patched DLL and for adding the error-mode tool to the patch.

    I'll commit and release a new version, after I add some documentation for the new feature.

    Regards,

    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2011-04-05

    • milestone: --> IINR_-_Include_In_Next_Release
     
  • Charles Wilson
    Charles Wilson
    2011-04-06

    NP. Don't forget to edit msysrlsbld.ini and add error-mode.exe to one of the packages. -ext, probably.

    I'm looking forward to the new release (and I can publish perl-5.8.8 soon after. Yay!)

     
  • Peter Rosin
    Peter Rosin
    2011-04-11

    Hi!

    I'm back. Sorry for the delay.

    While I haven't tested the error-mode-v2.patch (and not the integrated one either), it was one of the things I considered last year when I worked actively on the problem. I can't now remember why I didn't go for this simpler approach, so I have spent some time pondering why I elected to not do it this way. But I can't seem to remember my reasoning, which bugs me a bit...

    However, I wouldn't be surprised if it was just some paraniod moronic corner case. The v2-patch looks sane by occular examination and I trust the testing done by Chuck, so I too look forward to a release with this included!

    Cheers,
    Peter

     
  • Cesar Strauss
    Cesar Strauss
    2011-04-21

    Chuck wrote:
    > Don't forget to edit msysrlsbld.ini and add error-mode.exe
    > to one of the packages. -ext, probably.

    -bin is more appropriate, don't you think? -ext is mostly scripts, no executable so far. -bin already has some system tools like ps.exe and msysmnt.exe.

    Regards,
    Cesar

     
  • Cesar Strauss
    Cesar Strauss
    2011-09-18

    • status: open --> closed-accepted
     
  • Cesar Strauss
    Cesar Strauss
    2011-09-18

    This feature is now present in the 1.0.17 release.

    Thanks!

    Cesar