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

[Uli Schlachter]

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.

> +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.

> diff --git a/configure.ac b/configure.ac
> index ef300ad..8962156 100644
> --- a/configure.ac
> +++ b/configure.ac
[...]

Uli
-- 
“I’m Olaf and I like warm hugs.”