BUG #6166: configure from source fails with 'This platform is not thread-safe.' but was actually /tmp perms

"Alex Soto" <apsoto@gmail.com> writes:

I was trying to build the 9.0.4 source tarball as the postgres user on a
test machine.

The configure step failed with the error:
This platform is not thread-safe. Check the file 'config.log' or compile
and run src/test/thread/thread_test for the exact reason.
Use --disable-thread-safety to disable thread safety.

As I started looking through the log file I noticed that it failed to write
to the /tmp directory. Other configure steps had written to /var/tmp, but
this step tried to write to /tmp for some reason.

I fixed by correcting the permissions on the /tmp dir, but I thought the
error message was a little misleading, so I thought I'd report the problem.

Hmm ... I can't find any explicit reference to either /tmp or /var/tmp
in our configure script. It seems like this must be an artifact of your
compiler, or some other tool you're using.

regards, tom lane

Alex Soto <apsoto@gmail.com> writes:

Here's the section in the config.log in case it makes a difference

configure:28808: ./conftest
Could not create file in /tmp or
Could not generate failure for create file in /tmp **
exiting
configure:28812: $? = 1
configure: program exited with status 1

[ greps... ] Oh, that error is coming from src/test/thread/thread_test.c.

I wonder why we have that trying to write to /tmp at all, when all the
other transient junk generated by configure is in the current directory.
Bruce, do you have a good reason for doing it that way?

(The error message seems to be suffering from a bad case of copy-and-
paste-itis, too.)

regards, tom lane

Tom Lane wrote:

Alex Soto <apsoto@gmail.com> writes:

Here's the section in the config.log in case it makes a difference

configure:28808: ./conftest
Could not create file in /tmp or
Could not generate failure for create file in /tmp **
exiting
configure:28812: $? = 1
configure: program exited with status 1

[ greps... ] Oh, that error is coming from src/test/thread/thread_test.c.

I wonder why we have that trying to write to /tmp at all, when all the
other transient junk generated by configure is in the current directory.
Bruce, do you have a good reason for doing it that way?

No idea --- using the current directory is probably best.

(The error message seems to be suffering from a bad case of copy-and-
paste-itis, too.)

OK, let me see if I can fix this.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Tom Lane wrote:

Alex Soto <apsoto@gmail.com> writes:

Here's the section in the config.log in case it makes a difference

configure:28808: ./conftest
Could not create file in /tmp or
Could not generate failure for create file in /tmp **
exiting
configure:28812: $? = 1
configure: program exited with status 1

[ greps... ] Oh, that error is coming from src/test/thread/thread_test.c.

I wonder why we have that trying to write to /tmp at all, when all the
other transient junk generated by configure is in the current directory.
Bruce, do you have a good reason for doing it that way?

I have modified the code to use the current directory instead of /tmp.
I also cleaned up the #if defines and added some C comments.

(The error message seems to be suffering from a bad case of copy-and-
paste-itis, too.)

Actually, it is accurate. The code is:

#ifdef WIN32
h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL);
h2 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL);
if (h1 == INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
#else
if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0 ||
open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
#endif
{
fprintf(stderr, "Could not create file in current directory or\n");
fprintf(stderr, "could not generate failure for create file in current directory **\nexiting\n");
exit(1);
}

This code generates an errno == EEXIST in one thread, while another
thread generates errno == ENOENT, and this is how we test for errno
being thread-safe. If you have a cleaner way to do this, please let me
know. mkdir()?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

(The error message seems to be suffering from a bad case of copy-and-
paste-itis, too.)

Actually, it is accurate. The code is:

#ifdef WIN32
h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL);
h2 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL);
if (h1 == INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
#else
if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0 ||
open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
#endif
{
fprintf(stderr, "Could not create file in current directory or\n");
fprintf(stderr, "could not generate failure for create file in current directory **\nexiting\n");
exit(1);
}

This code generates an errno == EEXIST in one thread, while another
thread generates errno == ENOENT, and this is how we test for errno
being thread-safe. If you have a cleaner way to do this, please let me
know. mkdir()?

The problem with that is you're trying to make one error message serve
for two extremely different failure conditions. I think this should be
coded more like

if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0)
{
report suitable failure message;
exit(1);
}
if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
{
report suitable failure message;
exit(1);
}

You would probably find that the messages could be a lot more clear and
specific if they were done like that. Also, a file-related error
message that doesn't provide the filename nor strerror(errno) is pretty
much wrong on its face, in my book.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Tom Lane wrote:

(The error message seems to be suffering from a bad case of copy-and-
paste-itis, too.)

Actually, it is accurate. The code is:

#ifdef WIN32
h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL);
h2 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL);
if (h1 == INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
#else
if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0 ||
open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
#endif
{
fprintf(stderr, "Could not create file in current directory or\n");
fprintf(stderr, "could not generate failure for create file in current directory **\nexiting\n");
exit(1);
}

This code generates an errno == EEXIST in one thread, while another
thread generates errno == ENOENT, and this is how we test for errno
being thread-safe. If you have a cleaner way to do this, please let me
know. mkdir()?

The problem with that is you're trying to make one error message serve
for two extremely different failure conditions. I think this should be
coded more like

if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0)
{
report suitable failure message;
exit(1);
}
if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
{
report suitable failure message;
exit(1);
}

You would probably find that the messages could be a lot more clear and
specific if they were done like that. Also, a file-related error
message that doesn't provide the filename nor strerror(errno) is pretty
much wrong on its face, in my book.

OK, I split apart the two operations with the attached, applied patch.
There must be an easier way to generate two unique errno values in a
platform-indepentent way, but I can't find it. I did simplify the
second ENOENT generation by just doing unlink twice.

I can't just set errno to a different value, can I?

I am not able to use strerror because I don't know if that is
thread-safe yet. I do have a C comment about it:

/*
* strerror() might not be thread-safe, and we already spawned thread
* 1 that uses it, so avoid using it.
*/

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +