Mail Archives: cygwin/2014/08/07/07:52:26
Hi Corinna,
On 8/5/2014 2:40 PM, Corinna Vinschen wrote:
> Hi Ken,
>
> On Aug 5 13:55, Ken Brown wrote:
>> On 8/5/2014 9:58 AM, Corinna Vinschen wrote:
>>> On Aug 5 08:21, Ken Brown wrote:
>>>> === modified file 'src/gmalloc.c'
>>>> --- src/gmalloc.c 2014-03-04 19:02:49 +0000
>>>> +++ src/gmalloc.c 2014-08-05 01:35:38 +0000
>>>> @@ -490,8 +490,8 @@
>>>> }
>>>>
>>>> #ifdef USE_PTHREAD
>>>> -pthread_mutex_t _malloc_mutex = PTHREAD_MUTEX_INITIALIZER;
>>>> -pthread_mutex_t _aligned_blocks_mutex = PTHREAD_MUTEX_INITIALIZER;
>>>> +pthread_mutex_t _malloc_mutex;
>>>> +pthread_mutex_t _aligned_blocks_mutex;
>>>> int _malloc_thread_enabled_p;
>>>>
>>>> static void
>>>> @@ -526,8 +526,11 @@
>>>> initialized mutexes when they are used first. To avoid such a
>>>> situation, we initialize mutexes here while their use is
>>>> disabled in malloc etc. */
>>>> - pthread_mutex_init (&_malloc_mutex, NULL);
>>>> - pthread_mutex_init (&_aligned_blocks_mutex, NULL);
>>>> + pthread_mutexattr_t attr1, attr2;
>>>> + pthread_mutexattr_settype (&attr1, PTHREAD_MUTEX_NORMAL);
>>>> + pthread_mutexattr_settype (&attr2, PTHREAD_MUTEX_NORMAL);
>>>> + pthread_mutex_init (&_malloc_mutex, &attr1);
>>>> + pthread_mutex_init (&_aligned_blocks_mutex, &attr2);
>>>> pthread_atfork (malloc_atfork_handler_prepare,
>>>> malloc_atfork_handler_parent,
>>>> malloc_atfork_handler_child);
>>>>
>>>>
>>>> The first hunk avoids the double initialization, but I don't understand why
>>>> the second hunk does anything. Since PTHREAD_MUTEX_NORMAL is now the
>>>> default, shouldn't calling pthread_mutex_init with NULL second argument be
>>>> equivalent to my calls to pthread_mutexattr_settype? Does this indicate a
>>>> Cygwin bug, or am I misunderstanding something?
>>>
>>> AFAICS you're missing something. Your pthread_mutexattr_t attr1, attr2
>>> are not initialized. They contain some random values, thus they are not
>>> good objects. The calls to pthread_mutexattr_settype as well as the
>>> calls to pthread_mutex_init will fail with EINVAL, but you won't see it
>>> due to missing error handling, and you end up without mutexes at all.
>>> If you call pthread_mutexattr_init before calling
>>> pthread_mutexattr_settype the situation shoul;d be the same as before.
>>
>> Thanks for catching my mistake. Your earlier suggestion about explicitly
>> setting the pthread_mutexes to be ERRORCHECK mutexes seems to fix the
>> problem (as long as I remember to call pthread_mutexattr_init). The revised
>> patch is attached. I went back to using both the static and dynamic
>> initializations as in the original code, since you said that's harmless.
>
> I'm glad to read that, but I'm still a little bit concerned. If your
> code works with ERRORCHECK mutexes but hangs with NORMAL mutexes, you
> *might* miss an error case.
>
> I'd suggest to tweak the pthread_mutex_lock/unlock calls and log the
> threads calling it. It looks like the same thread calls malloc from
> malloc for some reason and it might be interesting to learn how that
> happens and if it's really ok in this scenario, because it seems to
> be unexpected by the code.
I think I found the problem with NORMAL mutexes. emacs calls
pthread_atfork after initializing the mutexes, and the resulting
'prepare' handler locks the mutexes. (The parent and child handlers
unlock them.) So when emacs calls fork, the mutexes are locked, and
shortly thereafter the Cygwin DLL calls calloc, leading to a deadlock.
Here's a gdb backtrace showing the sequence of calls:
#0 malloc (size=size AT entry=40) at gmalloc.c:919
#1 0x0053fc28 in calloc (nmemb=1, size=40) at gmalloc.c:1510
#2 0x61082074 in calloc (nmemb=1, size=40)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/malloc_wrapper.cc:100
#3 0x61003177 in operator new (s=s AT entry=40)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/cxx.cc:23
#4 0x610fc9d3 in pthread_mutex::init (mutex=0x61187d34 <reent_data+852>,
attr=0x0, initializer=0x12)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/thread.cc:3118
#5 0x610fcc13 in pthread_mutex_lock (mutex=0x61187d34 <reent_data+852>)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/thread.cc:3170
#6 0x611319d8 in __fp_lock (ptr=0x61187cd0 <reent_data+752>)
at /usr/src/debug/cygwin-1.7.31-3/newlib/libc/stdio/findfp.c:287
#7 0x61154f75 in _fwalk (ptr=0x28d544,
function=function AT entry=0x611319c0 <__fp_lock>)
at /usr/src/debug/cygwin-1.7.31-3/newlib/libc/stdio/fwalk.c:50
#8 0x61131dea in __fp_lock_all ()
at /usr/src/debug/cygwin-1.7.31-3/newlib/libc/stdio/findfp.c:307
#9 0x610fa45e in pthread::atforkprepare ()
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/thread.cc:2031
#10 0x61076292 in lock_pthread (this=<synthetic pointer>)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/sigproc.h:137
#11 hold_everything (x=<synthetic pointer>, this=<synthetic pointer>)
at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/sigproc.h:169
#12 fork () at /usr/src/debug/cygwin-1.7.31-3/winsup/cygwin/fork.cc:582
Is there a better way to deal with this issue than using ERRORCHECK mutexes?
Ken
--
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
- Raw text -