public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: cygwin-patches@cygwin.com
Subject: Re: [PATCH 1/5] Cygwin: Make 'ulimit -c' control writing a coredump
Date: Thu, 25 Jan 2024 19:21:55 +0100	[thread overview]
Message-ID: <ZbKmwy7wKjAJvF1u@calimero.vinschen.de> (raw)
In-Reply-To: <b140b902-8c5d-47f8-910e-f30d835bf185@dronecode.org.uk>

On Jan 25 14:50, Jon Turney wrote:
> On 24/01/2024 14:39, Corinna Vinschen wrote:
> > On Jan 24 13:28, Jon Turney wrote:
> > > On 23/01/2024 14:29, Corinna Vinschen wrote:
> > > > On Jan 23 14:20, Jon Turney wrote:
> > > > 
> > > > > Even then this is clearly not totally bullet-proof. Maybe the right thing to
> > > > > do is add a suitable timeout here, so even if we fail to notice the
> > > > > DebugActiveProcess() (or there's a custom JIT debugger which just writes the
> > > > > fact a process crashed to a logfile or something), we'll exit eventually?
> > > > 
> > > > Timeouts are just that tiny little bit more bullet-proof, they still
> > > > aren't totally bullet-proof.
> > > > 
> > > > What timeout were you thinking of?  milliseconds?
> > > 
> > > Oh no, tens of seconds or something, just as a fail-safe.
> > 
> > Uh, sounds a lot.  10 secs?  Not longer, I think.
> > 
> > If you want to do that for 3.5, please do it this week.  You can
> > push the change without waiting for approval.
> 
> Thanks.
> 
> I pushed a small change adding this timeout.
> 
> > > (Ofc, all this is working around the fact that Win32 API doesn't have a
> > > WaitForDebuggerPresent(timeout) function)
> > 
> > Yeah, and there's no alternative way using the native API afaics :(
> 
> So this situation with a JIT debugger is even stranger than my emendations
> to the documentation say.
> 
> Because we're hitting try_to_debug() in exception::handle(), which has some
> code to replay the exception via ExceptionContinueExecution (which I guess
> the debugger will catch as a first-chance) (and goes into a mode where it
> ignores the next half-million exceptions... no idea what that's about!)
> 
> That's not the same as signal_exit() with a coredumping signal (haven't
> checked if those are all generated from exceptions, but seemly probable, so
> the try_to_debug() there maybe isn't doing anything?), where we're going to
> exit thereafter.

try_to_debug() is only calling IsDebuggerPresent() as test, and that's
nothing but a flag in the PEB which is set by the OS after a debugger
attached to the process.  So the test is by definition extremely
flaky, if the debugger is connecting and disconnecting, as you
already pointed out.

I'm wondering if we can't define our own way to attach to a process,
allowing to "WaitForDebugger" as long as the debugger is a Cygwin
debugger.  If we define a matching function (along the lines of
prctl(2) on Linux), we could change our debuggers, core dumpers
and stracers to call this attach function.

The idea would be to define some shared mutex object, the inferior
waits for and the debugger releases after having attached.

Is there really any need to support non-Cygwin debuggers?

> The practical upshot of this is if the JIT debugger doesn't terminate or fix
> the erroring process, we'll just replay the faulting instruction and invoke
> the JIT debugger again.

Hmm, ok.  This signal stuff *is* complicated and I'd be happy
if anybody finds out how to fix that...


Corinna

  reply	other threads:[~2024-01-25 18:21 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-12 14:09 [PATCH 0/5] Coredump under 'ulimit -c' control (v2) Jon Turney
2024-01-12 14:09 ` [PATCH 1/5] Cygwin: Make 'ulimit -c' control writing a coredump Jon Turney
2024-01-13 14:20   ` Jon Turney
2024-01-15  9:46     ` Corinna Vinschen
2024-01-15 13:27       ` Jon Turney
2024-01-15 14:28         ` Corinna Vinschen
2024-01-16 13:52           ` Jon Turney
2024-01-16 13:54             ` Corinna Vinschen
2024-01-23 14:20   ` Jon Turney
2024-01-23 14:29     ` Corinna Vinschen
2024-01-24 13:28       ` Jon Turney
2024-01-24 14:39         ` Corinna Vinschen
2024-01-25 14:50           ` Jon Turney
2024-01-25 18:21             ` Corinna Vinschen [this message]
2024-01-25 20:03               ` Jon Turney
2024-01-26 11:12                 ` Corinna Vinschen
2024-01-26 11:52                   ` Corinna Vinschen
2024-01-27 15:12                     ` Jon Turney
2024-01-29 11:16                       ` Corinna Vinschen
2024-01-12 14:09 ` [PATCH 2/5] Cygwin: Disable writing core dumps by default Jon Turney
2024-01-12 14:09 ` [PATCH 3/5] Cygwin: Define and use __WCOREFLAG Jon Turney
2024-01-12 14:09 ` [PATCH 4/5] Cygwin: Treat api_fatal() similarly to a core-dumping signal Jon Turney
2024-01-12 14:09 ` [PATCH 5/5] Cygwin: Update documentation for cygwin_stackdump Jon Turney
2024-01-12 18:44   ` Corinna Vinschen
2024-01-13 13:40     ` Jon Turney
2024-01-12 18:41 ` [PATCH 0/5] Coredump under 'ulimit -c' control (v2) Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZbKmwy7wKjAJvF1u@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin-patches@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).