This is the mail archive of the
cygwin
mailing list for the Cygwin project.
Re: Instability with signals and threads
- From: Mikulas Patocka <mikulas at artax dot karlin dot mff dot cuni dot cz>
- To: Corinna Vinschen <corinna-cygwin at cygwin dot com>
- Cc: cygwin at cygwin dot com
- Date: Thu, 20 Nov 2014 21:22:31 +0100 (CET)
- Subject: Re: Instability with signals and threads
- Authentication-results: sourceware.org; auth=none
> Never mind that. I can fix your testcase by calling _my_tls.remove with
> INFINITE as parameter in both places. If I drop one of them, your
> testcase will invariable fail at one point. With both INFINITE params
> in place, your testcase is now running half an hour without problems.
For me, this change doesn't fix the testcase, it just reduces the
probability that it hangs.
With this change, the testcase still locks up, but with a different
stacktrace:
thread1:
Sleep
_yield
pthread::create
sigdelayed ??
_cygwin_exit_return ??
_cygtls::call2
thread2:
SetEvent
muto::release
init_cygheap::find_tls
_cygtls::init_thread
thread3:
WriteFile
sig_send
timer_thread
cygthread::callfunc
cygthread::stub
_cygtls::call2
thread4:
VirtualFree
thread_wrapper
thread5:
only ntdll stuff
So, apparently, there is another bug, where thread->cygtls isn't being set
and pthread::create loops endlessly calling yield.
> Thinking about it, the fact that _cygtls::remove allows to apply a
> non-INFINITE wait is rather strange, isn't it? Calling remove_tls with
> a 0 wait, it allows to return the function silently, without actually
> having removed the thread from the list. This is bound to go downhill
> at one point and looks like a kludge to me to circumvent some potential
> hang in another situation...
Looking at CVS history, the "wait" argument was added to cygtls.cc version
1.2 with a comment: "Add a 'wait' argument to control how long we wait for
a lock before removing." There is no explanation why is it needed.
> I'm not exactly sure if that works as intended. I will apply this patch
> and create a new Cygwin snapshot on https://cygwin.com/snapshots/ in a
> couple of minutes. I'd appreciate if you and others would give it an
> exhaustive test. New spurious hangs or SEGVs in other situations which
> so far worked fine would be good indicators for another problem in the
> code.
Yes, I think it's correct to remove the wait argument.
> Other than that, there's certainly some room for improvement. Calling
> threadlist[idx]->remove from the find_tls exception handler looks
> extremly hairy to me. I wonder if that should be called at all at this
> point, or if there shouldn't be better some "simplified" removal
> operation which doesn't require the _cygtls pointer. If the thread
> doesn't exist anymore, so does its _cygtls area.
I suggest to remove that exception handler at all. This thing can't ever
work reliably - it could reduce probability of crashes but not eliminate
them. Even if we handled the page fault correctly - what happens if some
other thread allocates a different object at the location that belonged to
the tls before? - then find_tls thinks that this different object is tls
and corrupts it.
I suggest to remove the exception handler and if it results in any
regressions, fix them properly.
Mikulas
> Thanks, Corinna --
> Corinna Vinschen
--
Problem reports: http://cygwin.com/problems.html
FAQ: http://cygwin.com/faq/
Documentation: http://cygwin.com/docs.html
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple