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

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. +
diff --git a/src/test/thread/thread_test.c b/src/test/thread/thread_test.c
new file mode 100644
index 2271ba6..6fef840
*** a/src/test/thread/thread_test.c
--- b/src/test/thread/thread_test.c
*************** main(int argc, char *argv[])
*** 263,268 ****
--- 263,271 ----
      }
  }

+ /*
+  * func_call_1
+  */
  static void
  func_call_1(void)
  {
*************** func_call_1(void)
*** 272,294 ****
      void       *p;
  #endif
  #ifdef WIN32
!     HANDLE        h1, h2;
  #endif

      unlink(TEMP_FILENAME_1);

      /* create, then try to fail on exclusive create open */
  #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);
      }

--- 275,312 ----
      void       *p;
  #endif
  #ifdef WIN32
!     HANDLE        h1;
! #else
!     int fd;
  #endif

      unlink(TEMP_FILENAME_1);

+     /* Set errno = EEXIST */
+
      /* create, then try to fail on exclusive create open */
  #ifdef WIN32
!     if ((h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL)) ==
!         INVALID_HANDLE_VALUE)
  #else
!     if ((fd = open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600)) < 0)
  #endif
      {
!         fprintf(stderr, "Could not create file %s in current directory\n",
!                 TEMP_FILENAME_1);
!         exit(1);
!     }
!
! #ifdef WIN32
!     if (CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL)
!         != INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
! #else
!     if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
! #endif
!     {
!         fprintf(stderr,
!                 "Could not generate failure for exclusive file create of %s in current directory **\nexiting\n",
!                 TEMP_FILENAME_1);
          exit(1);
      }

*************** func_call_1(void)
*** 315,320 ****
--- 333,343 ----
          exit(1);
      }

+ #ifdef WIN32
+     CloseHandle(h1);
+ #else
+     close(fd);
+ #endif
      unlink(TEMP_FILENAME_1);

  #ifndef HAVE_STRERROR_R
*************** func_call_1(void)
*** 352,357 ****
--- 375,383 ----
  }


+ /*
+  * func_call_2
+  */
  static void
  func_call_2(void)
  {
*************** func_call_2(void)
*** 363,377 ****

      unlink(TEMP_FILENAME_2);

!     /* open non-existant file */
! #ifdef WIN32
!     CreateFile(TEMP_FILENAME_2, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL);
!     if (GetLastError() != ERROR_FILE_NOT_FOUND)
! #else
!     if (open(TEMP_FILENAME_2, O_RDONLY, 0600) >= 0)
! #endif
      {
!         fprintf(stderr, "Read-only open succeeded without create **\nexiting\n");
          exit(1);
      }

--- 389,402 ----

      unlink(TEMP_FILENAME_2);

!     /* Set errno = ENOENT */
!
!     /* This will fail, but we can't check errno yet */
!     if (unlink(TEMP_FILENAME_2) != -1)
      {
!         fprintf(stderr,
!                 "Could not generate failure for unlink of %s in current directory **\nexiting\n",
!                 TEMP_FILENAME_2);
          exit(1);
      }

*************** func_call_2(void)
*** 394,405 ****
  #else
          fprintf(stderr, "errno not thread-safe **\nexiting\n");
  #endif
-         unlink(TEMP_FILENAME_2);
          exit(1);
      }

-     unlink(TEMP_FILENAME_2);
-
  #ifndef HAVE_STRERROR_R
      /*
       * If strerror() uses sys_errlist, the pointer might change for different
--- 419,427 ----