[Xcb] [PATCH pthread-stubs v2 3/4] Rework the pthread-stub design

[Emil Velikov]

On 4 November 2016 at 15:49, Uli Schlachter <psychon at znc.in> wrote:
> On 03.11.2016 21:42, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> The current design handles the most common use-cases, although it causes
>> breakage on others (when a pthreads liked library is dlopened). Refer to
>> the README for further details.
>>
>> The new design, makes pthread-stubs a "meta" package which _never_
>> provides a library but a simple .pc file.
>>
>> pthread-stubs checks if the run-time (libc or otherwise) expose
>> lightweight pthread symbols to link against and defaults to the full
>> blown pthread.
>>
>> This way projects can reuse the Cflags/Libs without having to know the
>> details. Alternatively they can directly link against the pthreads
>> implementation, although that might bring unwarranted overhead.
>>
>> v2: Use correct conditional in configure.ac
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  Makefile.am         |  10 --
>>  README              |  33 +++-
>>  configure.ac        |  64 +++----
>>  m4/ax_pthread.m4    | 485 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  pthread-stubs.pc.in |   3 +-
>>  stubs.c.m4          |  91 ----------
>>  6 files changed, 545 insertions(+), 141 deletions(-)
>>  create mode 100644 m4/ax_pthread.m4
>>  delete mode 100644 stubs.c.m4
>>
> [...]
>> diff --git a/README b/README
>> index 4e7e85b..44f39ee 100644
>> --- a/README
>> +++ b/README
>> @@ -1,9 +1,24 @@
>> -This library provides weak aliases for pthread functions not provided in libc
>> -or otherwise available by default.  Libraries like libxcb rely on pthread
>> -stubs to use pthreads optionally, becoming thread-safe when linked to
>> -libpthread, while avoiding any performance hit when running single-threaded.
>> -libpthread-stubs supports this behavior even on platforms which do not supply
>> -all the necessary pthread stubs.  On platforms which already supply all the
>> -necessary pthread stubs, this package ships only the pkg-config file
>> -pthread-stubs.pc, to allow libraries to unconditionally express a dependency
>> -on pthread-stubs and still obtain correct behavior.
>> +Originally this library used to provide weak aliases for pthread functions not
>> +provided in libc or otherwise available by default.
>> +
>> +This was found to be a fundamental issue with the design, as the following
>> +scenario came to light:
>> +
>> +Program/library links against pthread-stubs and none of its direct dependencies
>> +link against pthreads. Thus the weak pthread symbols are used. At a later stage
>> +a module is dlopened. A one which (directly or via some of its dependencies)
>
> I fail to parse "A one" here.
>
"A module"

>> +pulls a full-fledged pthreads implementation. This leads to the weak symbols
>> +being overridden thus any consecutive calls to the pthreads API [by the
>> +pthread-stubs user] will be mismatched leading to all sorts of issues.
>> +
>> +In order to prevent that one has to traverse up the dependency chain (from the
>
> I'd propose to add a comma after "that", since I first read this as "In
> order to prevent that one, has".
>
>> +pthread-stubs user) and establish whether any of the dependencies will dlopen
>> +a module which pulls the complete pthread library.
>> +
>> +As this is not deemed practical and the alternative solution implemented here
>
> The "and" seems wrong, perhaps replace with ","?
>
>> +is as follows:
>> +
>> +The pthread-stubs package only ever provided a .pc file, which provides a set
>
> provided -> provides
>
>> +of Cflags and Libs.
>> + - In the case of pthreads capable libc those are empty
>> + - Alternatively they expand to the platform specific PTHREAD_{CFLAGS,LIBS}
>
> Perhaps the README should start with an explanation of the current state
> and the history only follows later? Most users would likely want to know
> which symbols and definitions are guaranteed by this library.
>
> Perhaps the list from configure.ac should also be mentioned her? After
> all, people would want to know which parts of pthreads are
> provided/guaranteed by pthread-stubs.
>
Sure thing, can reshuffle things and apply some polish.

Thanks
Emil