[PATCH] pthread_detach: add a comment about a known segfault bug.

[Florian Weimer]

* Paul Pluzhnikov:

>> The trick to make the test start a new iteration triggers the EINVAL
>> failure much more frequently:
>>
>> error: xpthread_check_return.c:32: pthread_detach: Invalid argument
>> *** stack smashing detected ***: terminated
>
> What is your observed failure rate?

I didn't set up good monitoring because I didn't expect to reproduce the
issue.  With the accelerated version (reading /proc/self/task) I saw
around 7 failures in perhaps 50,000 test iterations.

I'm running the patched version against Adhemerval's fix over night.  So
far there have not been any failures.  Not a replacement for proper
patch review, of course.  I think it's useful as an additional data
point.

> I modified the test to run 1,000,000 iterations, also changed it to

I decided against bumping the iteration count.

> not sleep when there are no remaining tasks, and ran 20 instances of
> that binary overnight (with --direct to avoid timeout) on a single
> machine against the ToT GLIBC build.
> None of them crashed, and they reached 400,000+ iterations.

In some previous cases, we definitely saw that the dice were rolled at
process start: increasing the test iteration count did not make it
significantly more likely that the failure would occur in this specific
processes.  So doing fewer execve calls actually reduced reproduction
rate.  I suppose this could happen due to ASLR, NUMA placement, or
similar memory effects.  For this test case, it might also be relevant
how the glibc stack cache gets configured during the first few
iterations of the test.

>> /* Returns the number of threads in this process, by counting the
>>    entries in /proc/self/task.  */
>> static int
>> current_thread_count (void)
>> {
>>   DIR *tasks = xopendir ("/proc/self/task");
>>   int count = 0;
>>   struct dirent *d;
>>   while ((d = xreaddir (tasks)) != NULL)
>>     if (d->d_name[0] != '.')
>>       ++count;
>>   xclosedir (tasks);
>>   return count;
>> }
>
> I did that part as:
>
> static int
> ntasks (void)
> {
>   struct stat st_buf;
>   xstat ("/proc/self/task", &st_buf);
>   return st_buf.st_nlink - 2;
> }

Given that st_nlink is not correct in the first place, I wouldn't rely
on it for this. 8-/

Thanks,
Florian