Bug 35122

Thanks for this bug report.  Could you attach the full output file config.log of configure before applying your patch?  The default check should work if no libs are needed, so linking explicitly with libc seems superfluous.  We've had a number of problems with out flag checking code being too strict for some platforms (as a reaction to not being strict enough on others.)

Created attachment 44275 [details]
config.log as requested

config.log as requested.  The issue appears to be
in cairo-1.10.0/build/configure.ac.pthread which
defaults PTHREAD_LIBS="-lpthread" if not already
set.

(In reply to comment #2)

> config.log as requested.  The issue appears to be
> in cairo-1.10.0/build/configure.ac.pthread which
> defaults PTHREAD_LIBS="-lpthread" if not already
> set.

Yet, according to config.log, it correctly figures out that no extra pthread libraries need to be linked in.  From the log:

> pthread_CFLAGS='-D_REENTRANT'
> pthread_LIBS=''
[snip]
> real_pthread_CFLAGS=''
> real_pthread_LIBS=''
[snip]
> #define CAIRO_HAS_PTHREAD 1

Further up in the log it tries to use -lpthread, but that doesn't work, and then it tries -pthread, but that also doesn't work, and finally it tries nothing at all, and that does work.

The log also shows that configure.ac.pthreads doesn't believe your libc has "real" pthreads as opposed to stubs, but that's okay since compiling the library proper doesn't need "real" pthread functionality. The test suite does, however, as some of the tests spawn actual threads, so make check might not work for you.

What is the actual error you're seeing?

The issue is this section:

configure:33670: checking whether cairo's cairo-trace feature could be enabled
configure:33674: result: no (requires dynamic linker and zlib and real pthreads)

With the patch this passes and I also get:

#define CAIRO_HAS_REAL_PTHREAD 1
#define CAIRO_HAS_TRACE 1

(In reply to comment #4)
> The issue is this section:
> 
> configure:33670: checking whether cairo's cairo-trace feature could be enabled
> configure:33674: result: no (requires dynamic linker and zlib and real
> pthreads)

Right, you did say that it was to get the trace feature working. Okay, so
here's what I think is happening:  cairo-trace needs real pthreads for
pthread_once() and friends, but our configure test doesn't think libc provides
for real pthreads. This is because many other platforms have weakly linked stub
implementations of the pthread primitives for efficiency reasons in case the
application isn't actually multithreaded.  A pure link test against libc
doesn't work on those platforms as it will give a false positive signal for
pthreads being available in libc when in reality the libc implementation will
error out when it comes time to do something actually thready.  This makes it
impossible to test for real pthread support in libc without actually running
some code and seeing if it fails.

Your patch works around the configuring logic by making it believe that the
user set PTHREAD_LIBS. Our basic bug there is that we consider
PTHREAD_LIBS/CFLAGS=""
in the environment to be unset, even though the user has explicitly told us not
to use any flags or libs at all.  Our deeper bug is that we don't trust libc to
implement pthreads properly, but I'm not sure we can fix that without some old
fashioned "if <your OS>" conditionals.

The reason cairo-trace utility needs to use real pthreads because it would be a
disaster if the application were actually threaded, but cairo trace used libc
stubs (this may be an issue due to cairo-trace LD_PRELOADING the interposer
libcairo-tracer.so.)  On the other hand, using cairo-trace on a threaded
application seems to be a disaster anyway right now, so changing the
cairo-trace requirement from real pthreads might not make a practical
difference. :)

(In reply to comment #5)
> (In reply to comment #4)
> > The issue is this section:
> > 
> > configure:33670: checking whether cairo's cairo-trace feature could be enabled
> > configure:33674: result: no (requires dynamic linker and zlib and real
> > pthreads)
> 
> Right, you did say that it was to get the trace feature working. Okay, so
> here's what I think is happening:  cairo-trace needs real pthreads for
> pthread_once() and friends, but our configure test doesn't think libc provides
> for real pthreads. This is because many other platforms have weakly linked stub
> implementations of the pthread primitives for efficiency reasons in case the
> application isn't actually multithreaded.  A pure link test against libc
> doesn't work on those platforms as it will give a false positive signal for
> pthreads being available in libc when in reality the libc implementation will
> error out when it comes time to do something actually thready.  This makes it
> impossible to test for real pthread support in libc without actually running
> some code and seeing if it fails.
> 
> Your patch works around the configuring logic by making it believe that the
> user set PTHREAD_LIBS. Our basic bug there is that we consider
> PTHREAD_LIBS/CFLAGS=""
> in the environment to be unset, even though the user has explicitly told us not
> to use any flags or libs at all.  Our deeper bug is that we don't trust libc to
> implement pthreads properly, but I'm not sure we can fix that without some old
> fashioned "if <your OS>" conditionals.
> 
> The reason cairo-trace utility needs to use real pthreads because it would be a
> disaster if the application were actually threaded, but cairo trace used libc
> stubs (this may be an issue due to cairo-trace LD_PRELOADING the interposer
> libcairo-tracer.so.)  On the other hand, using cairo-trace on a threaded
> application seems to be a disaster anyway right now, so changing the
> cairo-trace requirement from real pthreads might not make a practical
> difference. :)

So does that mean you'll accept the patch?

(In reply to comment #6)

> So does that mean you'll accept the patch?

Not quite. The patch is a diff against configure, but that's an autotools generated file from configure.ac and various build/* files.  I can use it as a basis for fixing it well enough though.  I'll report back when there's something to test. Until then I hope you can live with the workaround of setting PTHREAD_LIBS to something.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/170.