[PATCH v3] nptl: Deallocate the thread stack on setup failure (BZ #19511)
Adhemerval Zanella
adhemerval.zanella@linaro.org
Wed Jun 9 17:07:02 GMT 2021
On 09/06/2021 10:49, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> THREAD_RAN only signals if whether clone() or thread setup has failed,
>> there is no synchronization involved and it is already local to
>> creating thread. I changed the concurrency comment to the following,
>> to indicate that (c) and (d) are essentially the same as you noted:
>>
>> (c) If either a joinable or detached thread setup failed and THREAD_RAN
>> is true, then the creating thread releases ownership to the new thread,
>> the created thread sees the failed setup through PD SETUP_FAILED
>
> PD->setup_failed (I think this is the GNU convention).
Ack (I was in doubt which would the rule here).
>
>> member, releases the PD ownership, and exits. The creating thread will
>> be responsible for cleanup the allocated resources. The THREAD_RAN is
>> local to creating thread and indicate whether thread creation or setup
>> has failed.
>>
>> (d) If the thread creation failed and THREAD_RAN is false (meaning
>> ARCH_CLONE has failed), then the creating thread retains ownership
>> of PD and must cleanup he allocated resource. No waiting for the new
>> thread is required because it never started.
>
> Yes, that's much better.
>
>> I think for such case you will need to set PD->stopped_start to true
>> before thread creation (similar to what affinity and schedule set do),
>> add the required initialization after the setup failure check, and
>> handle potential errors.
>>
>> Something like (I removed the comments to simplify):
>
> Yeah, that is what I expect as well.
>
>> Below it is the updated patch based on your suggestion, including the
>> concurrency state descriptor regarding THREAD_RAN. Are this version
>> ok to commit?
>
> This version looks okay to me, with the PD->setup_failed change above.
Thanks, I will run a check on some architecture and commit the patchset.
More information about the Libc-alpha
mailing list