Re: (call-process ...) hangs in emacs

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

[-- Attachment #1: Type: text/plain, Size: 3998 bytes --]

On Aug  5 08:21, Ken Brown wrote:
> On 8/4/2014 9:45 AM, Corinna Vinschen wrote:
> >...this, and the fact that fork/exec (vfork == fork on Cygwin) still
> >works nicely in other scenarios points to some problem with the usage of
> >pthread_mutexes in the application may be the culprit.
> >
> >For instance, is it possible that emacs expects the pthread_mutexes
> >in malloc to be ERRORCHECK mutexes?  What if you explicitely set
> >them to ERRORCHECK at creation time?
> 
> That doesn't seem to be the issue, but I think I did find the problem, and
> it looks like there might be both an emacs bug and a Cygwin bug. Here's the
> relevant code from emacs's gmalloc.c:
> 
> pthread_mutex_t _malloc_mutex = PTHREAD_MUTEX_INITIALIZER;
> pthread_mutex_t _aligned_blocks_mutex = PTHREAD_MUTEX_INITIALIZER;
> 
> [...]
> 
>   /* Some pthread implementations call malloc for statically
>      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);
> 
> 
> The pthread_mutexes are initialized twice, resulting in undefined behavior
> according to Posix.  That's the emacs bug.

That's not the problem.  It's not necessary to call pthread_mutex_init
on statically initialized mutexes, but it doesn't hurt either.  Only
when calling pthread_mutex_init twice on the same object it goes
downhill, especially when the first incarnation of the mutex was already
locked.

> But simply removing the static
> initialization doesn't fix the problem.  On the other hand, the following
> patch does seem to fix it, at least in preliminary testing:
> 
> === 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.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]