Mail Archives: cygwin/2014/08/05/13:55:59
--------------020304080105040401080000
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
On 8/5/2014 9:58 AM, Corinna Vinschen wrote:
> 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.
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.
Angelo and Katsumi, could you test it and see if it solves the problems
you reported? If so, I'll issue new emacs releases.
Ken
--------------020304080105040401080000
Content-Type: text/plain; charset=windows-1252;
name="pthread_mutex.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
filename="pthread_mutex.patch"
PT09IG1vZGlmaWVkIGZpbGUgJ3NyYy9nbWFsbG9jLmMnCi0tLSBzcmMvZ21h
bGxvYy5jCTIwMTQtMDMtMDQgMTk6MDI6NDkgKzAwMDAKKysrIHNyYy9nbWFs
bG9jLmMJMjAxNC0wOC0wNSAxNzozMDoxOCArMDAwMApAQCAtNDkwLDggKzQ5
MCw4IEBACiB9CiAKICNpZmRlZiBVU0VfUFRIUkVBRAotcHRocmVhZF9tdXRl
eF90IF9tYWxsb2NfbXV0ZXggPSBQVEhSRUFEX01VVEVYX0lOSVRJQUxJWkVS
OwotcHRocmVhZF9tdXRleF90IF9hbGlnbmVkX2Jsb2Nrc19tdXRleCA9IFBU
SFJFQURfTVVURVhfSU5JVElBTElaRVI7CitwdGhyZWFkX211dGV4X3QgX21h
bGxvY19tdXRleCA9IFBUSFJFQURfRVJST1JDSEVDS19NVVRFWF9JTklUSUFM
SVpFUl9OUDsKK3B0aHJlYWRfbXV0ZXhfdCBfYWxpZ25lZF9ibG9ja3NfbXV0
ZXggPSBQVEhSRUFEX0VSUk9SQ0hFQ0tfTVVURVhfSU5JVElBTElaRVJfTlA7
CiBpbnQgX21hbGxvY190aHJlYWRfZW5hYmxlZF9wOwogCiBzdGF0aWMgdm9p
ZApAQCAtNTI2LDggKzUyNiwxMyBAQAogICAgICBpbml0aWFsaXplZCBtdXRl
eGVzIHdoZW4gdGhleSBhcmUgdXNlZCBmaXJzdC4gIFRvIGF2b2lkIHN1Y2gg
YQogICAgICBzaXR1YXRpb24sIHdlIGluaXRpYWxpemUgbXV0ZXhlcyBoZXJl
IHdoaWxlIHRoZWlyIHVzZSBpcwogICAgICBkaXNhYmxlZCBpbiBtYWxsb2Mg
ZXRjLiAgKi8KLSAgcHRocmVhZF9tdXRleF9pbml0ICgmX21hbGxvY19tdXRl
eCwgTlVMTCk7Ci0gIHB0aHJlYWRfbXV0ZXhfaW5pdCAoJl9hbGlnbmVkX2Js
b2Nrc19tdXRleCwgTlVMTCk7CisgIHB0aHJlYWRfbXV0ZXhhdHRyX3QgYXR0
cjEsIGF0dHIyOworICBwdGhyZWFkX211dGV4YXR0cl9pbml0ICgmYXR0cjEp
OworICBwdGhyZWFkX211dGV4YXR0cl9pbml0ICgmYXR0cjIpOworICBwdGhy
ZWFkX211dGV4YXR0cl9zZXR0eXBlICgmYXR0cjEsIFBUSFJFQURfTVVURVhf
RVJST1JDSEVDSyk7CisgIHB0aHJlYWRfbXV0ZXhhdHRyX3NldHR5cGUgKCZh
dHRyMiwgUFRIUkVBRF9NVVRFWF9FUlJPUkNIRUNLKTsKKyAgcHRocmVhZF9t
dXRleF9pbml0ICgmX21hbGxvY19tdXRleCwgJmF0dHIxKTsKKyAgcHRocmVh
ZF9tdXRleF9pbml0ICgmX2FsaWduZWRfYmxvY2tzX211dGV4LCAmYXR0cjIp
OwogICBwdGhyZWFkX2F0Zm9yayAobWFsbG9jX2F0Zm9ya19oYW5kbGVyX3By
ZXBhcmUsCiAJCSAgbWFsbG9jX2F0Zm9ya19oYW5kbGVyX3BhcmVudCwKIAkJ
ICBtYWxsb2NfYXRmb3JrX2hhbmRsZXJfY2hpbGQpOwoK
--------------020304080105040401080000
Content-Type: text/plain; charset=us-ascii
--
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
--------------020304080105040401080000--
- Raw text -