diff mbox series

[v2] Add freopen special-case tests: thread cancellation

Message ID a9aceb15-5d50-be10-e45b-9be5c6ae13f3@redhat.com
State New
Headers show
Series [v2] Add freopen special-case tests: thread cancellation | expand

Commit Message

Joseph Myers Sept. 12, 2024, 10:43 p.m. UTC
On Thu, 12 Sep 2024, Florian Weimer wrote:

> * Joseph Myers:
> 
> > Note that it's in the nature of the uncertain time at which
> > cancellation might act (possibly during freopen, possibly during
> > subsequent reads) that these can leak memory or file descriptors, so
> > these do not include leak tests.
> 
> Hmm.  Is it even possible for applications to handle this correctly?
> Given that freopen closes the original stream on error?  So the
> cancellation handler can't know if the stream is still open or not.

I don't expect this to work particularly well in applications (in terms of 
knowing if the stream is open).

> > +ifeq ($(subdir),stdio-common)
> > +tests += \
> > +  tst-freopen-cancel \
> > +  tst-freopen64-cancel \
> > +  # tests
> > +
> > +$(objpfx)tst-freopen-cancel: $(shared-thread-library)
> > +$(objpfx)tst-freopen64-cancel: $(shared-thread-library)
> > +endif
> 
> The test could be in stdio-common.  It doesn't need serial execution.

I've moved the test.  This makes it depend on my previous patch with other 
special-case tests (textually in the Makefile, not in any substantive 
way).

> > +  fp2 = xfopen (file3, "wc");
> > +  fputs ("rc_to_r got to freopen", fp2);
> > +  xfclose (fp2);
> > +  /* Cancellation should occur at some point from here onwards
> > +     (possibly leaking memory and file descriptors associated with the
> > +     FILE).  */
> > +  fp = FREOPEN (file2, "r", fp);
> > +  TEST_VERIFY_EXIT (fp != NULL);
> > +  for (;;)
> > +    {
> > +      fgetc (fp);
> > +      fseek (fp, 0, SEEK_SET);
> > +    }
> > +}
> 
> The test does not assert that cancellation happens in the expected
> region.  You could set up fp in the main thread and pass it via the
> test_rc_to_r pointer argument?

It makes sure cancellation only occurs after the checked-for text is 
written to file3.

> > +void *
> > +test_r_to_rc (void *p)
> > +{
> > +  int ret;
> > +  FILE *fp;
> > +  fp = xfopen (file1, "r");
> > +  fp = FREOPEN (file2, "rc", fp);
> > +  TEST_VERIFY_EXIT (fp != NULL);
> > +  ret = sem_post (&sem);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  /* No cancellation should occur for I/O on file2.  */
> > +  for (int i = 0; i < 1000000; i++)
> > +    {
> > +      fgetc (fp);
> > +      fseek (fp, 0, SEEK_SET);
> > +    }
> 
> Maybe you could use a FIFO and <support/process_state.h> to avoid
> spinning here?  Call pthread_cancel only after
> support_process_state_wait reports reaching
> support_process_state_sleeping?

I'm not sure a check for sleeping is right, especially since we're 
concerned about the state of a thread not a process.  But I've changed to 
use a fifo and avoided spinning that way.

> > +  xfclose (fp);
> > +  fp = xfopen (file3, "wc");
> > +  fputs ("r_to_rc got to fclose", fp);
> > +  xfclose (fp);
> > +  for (;;)
> > +    pthread_testcancel ();
> > +}
> 
> I assume you wrote it this way to assert that there actually is a
> pending cancellation request.  I suggest to set a global variable after
> the xfclose to make sure that the cancellation was not acted upon in
> fclose.

If cancellation occurred in the first xfclose, then the check for file3 
contents would fail.  There's an implicit assumption that "c" works OK 
with fopen so the second xfclose isn't an issue (just changes in freopen 
are being tested here).


Add freopen special-case tests: thread cancellation

Add tests of freopen adding or removing "c" (non-cancelling I/O) from
the mode string (so completing my planned tests of freopen with
different features used in the mode strings).  Note that it's in the
nature of the uncertain time at which cancellation might act (possibly
during freopen, possibly during subsequent reads) that these can leak
memory or file descriptors, so these do not include leak tests.

Tested for x86_64.

---

Changed in v2: moved tests to stdio-common; use a fifo in test_r_to_rc
to avoid any need to spin reading or calling pthread_testcancel.

Comments

Joseph Myers Sept. 20, 2024, 9:28 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/pipermail/libc-alpha/2024-September/159919.html> 
is pending review.
Adhemerval Zanella Netto Oct. 1, 2024, 12:59 p.m. UTC | #2
On 12/09/24 19:43, Joseph Myers wrote:
> On Thu, 12 Sep 2024, Florian Weimer wrote:
> 
>> * Joseph Myers:
>>
>>> Note that it's in the nature of the uncertain time at which
>>> cancellation might act (possibly during freopen, possibly during
>>> subsequent reads) that these can leak memory or file descriptors, so
>>> these do not include leak tests.
>>
>> Hmm.  Is it even possible for applications to handle this correctly?
>> Given that freopen closes the original stream on error?  So the
>> cancellation handler can't know if the stream is still open or not.
> 
> I don't expect this to work particularly well in applications (in terms of 
> knowing if the stream is open).
> 
>>> +ifeq ($(subdir),stdio-common)
>>> +tests += \
>>> +  tst-freopen-cancel \
>>> +  tst-freopen64-cancel \
>>> +  # tests
>>> +
>>> +$(objpfx)tst-freopen-cancel: $(shared-thread-library)
>>> +$(objpfx)tst-freopen64-cancel: $(shared-thread-library)
>>> +endif
>>
>> The test could be in stdio-common.  It doesn't need serial execution.
> 
> I've moved the test.  This makes it depend on my previous patch with other 
> special-case tests (textually in the Makefile, not in any substantive 
> way).
> 
>>> +  fp2 = xfopen (file3, "wc");
>>> +  fputs ("rc_to_r got to freopen", fp2);
>>> +  xfclose (fp2);
>>> +  /* Cancellation should occur at some point from here onwards
>>> +     (possibly leaking memory and file descriptors associated with the
>>> +     FILE).  */
>>> +  fp = FREOPEN (file2, "r", fp);
>>> +  TEST_VERIFY_EXIT (fp != NULL);
>>> +  for (;;)
>>> +    {
>>> +      fgetc (fp);
>>> +      fseek (fp, 0, SEEK_SET);
>>> +    }
>>> +}
>>
>> The test does not assert that cancellation happens in the expected
>> region.  You could set up fp in the main thread and pass it via the
>> test_rc_to_r pointer argument?
> 
> It makes sure cancellation only occurs after the checked-for text is 
> written to file3.
> 
>>> +void *
>>> +test_r_to_rc (void *p)
>>> +{
>>> +  int ret;
>>> +  FILE *fp;
>>> +  fp = xfopen (file1, "r");
>>> +  fp = FREOPEN (file2, "rc", fp);
>>> +  TEST_VERIFY_EXIT (fp != NULL);
>>> +  ret = sem_post (&sem);
>>> +  TEST_VERIFY_EXIT (ret == 0);
>>> +  /* No cancellation should occur for I/O on file2.  */
>>> +  for (int i = 0; i < 1000000; i++)
>>> +    {
>>> +      fgetc (fp);
>>> +      fseek (fp, 0, SEEK_SET);
>>> +    }
>>
>> Maybe you could use a FIFO and <support/process_state.h> to avoid
>> spinning here?  Call pthread_cancel only after
>> support_process_state_wait reports reaching
>> support_process_state_sleeping?
> 
> I'm not sure a check for sleeping is right, especially since we're 
> concerned about the state of a thread not a process.  But I've changed to 
> use a fifo and avoided spinning that way.
> 
>>> +  xfclose (fp);
>>> +  fp = xfopen (file3, "wc");
>>> +  fputs ("r_to_rc got to fclose", fp);
>>> +  xfclose (fp);
>>> +  for (;;)
>>> +    pthread_testcancel ();
>>> +}
>>
>> I assume you wrote it this way to assert that there actually is a
>> pending cancellation request.  I suggest to set a global variable after
>> the xfclose to make sure that the cancellation was not acted upon in
>> fclose.
> 
> If cancellation occurred in the first xfclose, then the check for file3 
> contents would fail.  There's an implicit assumption that "c" works OK 
> with fopen so the second xfclose isn't an issue (just changes in freopen 
> are being tested here).
> 
> 
> Add freopen special-case tests: thread cancellation
> 
> Add tests of freopen adding or removing "c" (non-cancelling I/O) from
> the mode string (so completing my planned tests of freopen with
> different features used in the mode strings).  Note that it's in the
> nature of the uncertain time at which cancellation might act (possibly
> during freopen, possibly during subsequent reads) that these can leak
> memory or file descriptors, so these do not include leak tests.
> 
> Tested for x86_64.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
> 
> Changed in v2: moved tests to stdio-common; use a fifo in test_r_to_rc
> to avoid any need to spin reading or calling pthread_testcancel.
> 
> diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> index 62f8b99b06..79da56d055 100644
> --- a/stdio-common/Makefile
> +++ b/stdio-common/Makefile
> @@ -222,10 +222,12 @@ tests := \
>    tst-freopen4 \
>    tst-freopen5 \
>    tst-freopen6 \
> +  tst-freopen7 \
>    tst-freopen64-2 \
>    tst-freopen64-3 \
>    tst-freopen64-4 \
>    tst-freopen64-6 \
> +  tst-freopen64-7 \
>    tst-fseek \
>    tst-fwrite \
>    tst-fwrite-memstrm \
> @@ -620,3 +622,6 @@ $(objpfx)tst-setvbuf1-cmp.out: tst-setvbuf1.expect $(objpfx)tst-setvbuf1.out
>  
>  $(objpfx)tst-printf-round: $(libm)
>  $(objpfx)tst-scanf-round: $(libm)
> +
> +$(objpfx)tst-freopen7: $(shared-thread-library)
> +$(objpfx)tst-freopen64-7: $(shared-thread-library)
> diff --git a/stdio-common/tst-freopen64-7.c b/stdio-common/tst-freopen64-7.c
> new file mode 100644
> index 0000000000..f34c280521
> --- /dev/null
> +++ b/stdio-common/tst-freopen64-7.c
> @@ -0,0 +1,2 @@
> +#define FREOPEN freopen64
> +#include <tst-freopen7-main.c>
> diff --git a/stdio-common/tst-freopen7-main.c b/stdio-common/tst-freopen7-main.c
> new file mode 100644
> index 0000000000..965e0b4adc
> --- /dev/null
> +++ b/stdio-common/tst-freopen7-main.c
> @@ -0,0 +1,155 @@
> +/* Test freopen cancellation handling.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <mcheck.h>
> +#include <pthread.h>
> +#include <semaphore.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <wchar.h>
> +
> +#include <support/check.h>
> +#include <support/file_contents.h>
> +#include <support/support.h>
> +#include <support/temp_file.h>
> +#include <support/test-driver.h>
> +#include <support/xstdio.h>
> +#include <support/xthread.h>
> +#include <support/xunistd.h>
> +
> +char *file1, *file2, *file3, *fifo;
> +
> +sem_t sem;
> +
> +void *
> +test_rc_to_r (void *p)
> +{
> +  int ret;
> +  FILE *fp, *fp2;
> +  ret = sem_post (&sem);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  fp = xfopen (file1, "rc");
> +  for (int i = 0; i < 1000000; i++)
> +    {
> +      fgetc (fp);
> +      fseek (fp, 0, SEEK_SET);
> +    }
> +  fp2 = xfopen (file3, "wc");
> +  fputs ("rc_to_r got to freopen", fp2);
> +  xfclose (fp2);
> +  /* Cancellation should occur at some point from here onwards
> +     (possibly leaking memory and file descriptors associated with the
> +     FILE).  */
> +  fp = FREOPEN (file2, "r", fp);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  for (;;)
> +    {
> +      fgetc (fp);
> +      fseek (fp, 0, SEEK_SET);
> +    }
> +}
> +
> +void *
> +test_r_to_rc (void *p)
> +{
> +  int ret;
> +  FILE *fp;
> +  fp = xfopen (file1, "r");
> +  fp = FREOPEN (fifo, "rc", fp);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  ret = sem_post (&sem);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  /* No cancellation should occur for I/O on fifo.  */
> +  ret = fgetc (fp);
> +  /* At this point, the other thread has called pthread_cancel and
> +     then written a byte to the fifo, so this thread is cancelled at
> +     the next cancellation point.  */
> +  TEST_VERIFY (ret == 'x');
> +  xfclose (fp);
> +  fp = xfopen (file3, "wc");
> +  fputs ("r_to_rc got to fclose", fp);
> +  xfclose (fp);
> +  pthread_testcancel ();
> +  FAIL_EXIT1 ("test_r_to_rc not cancelled\n");
> +}
> +
> +int
> +do_test (void)
> +{
> +  char *temp_dir = support_create_temp_directory ("tst-freopen-cancel");
> +  file1 = xasprintf ("%s/file1", temp_dir);
> +  support_write_file_string (file1, "file1");
> +  add_temp_file (file1);
> +  file2 = xasprintf ("%s/file2", temp_dir);
> +  support_write_file_string (file2, "file2");
> +  add_temp_file (file2);
> +  file3 = xasprintf ("%s/file3", temp_dir);
> +  support_write_file_string (file3, "file3");
> +  add_temp_file (file3);
> +  fifo = xasprintf ("%s/fifo", temp_dir);
> +  xmkfifo (fifo, 0666);
> +  add_temp_file (fifo);
> +  int ret;
> +  pthread_t thr;
> +  void *retval;
> +
> +  /* Test changing to/from c (cancellation disabled).  */
> +
> +  verbose_printf ("Testing rc -> r\n");
> +  ret = sem_init (&sem, 0, 0);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  thr = xpthread_create (NULL, test_rc_to_r, NULL);
> +  ret = sem_wait (&sem);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  xpthread_cancel (thr);
> +  ret = pthread_join (thr, &retval);
> +  TEST_COMPARE (ret, 0);
> +  TEST_VERIFY (retval == PTHREAD_CANCELED);
> +  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "rc_to_r got to freopen");
> +
> +  verbose_printf ("Testing r -> rc\n");
> +  ret = sem_init (&sem, 0, 0);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  thr = xpthread_create (NULL, test_r_to_rc, NULL);
> +  FILE *fp = xfopen (fifo, "w");
> +  ret = sem_wait (&sem);
> +  TEST_VERIFY_EXIT (ret == 0);
> +  /* This call happens while, or before, the other thread is waiting
> +     to read a character from the fifo.  It thus verifies that
> +     cancellation does not occur from the fgetc call in that thread
> +     (it should instead occur only in pthread_testcancel call),
> +     because the expected string is only written to file3 after that
> +     thread closes the fifo.  */
> +  xpthread_cancel (thr);
> +  fputc ('x', fp);
> +  xfclose (fp);
> +  ret = pthread_join (thr, &retval);
> +  TEST_COMPARE (ret, 0);
> +  TEST_VERIFY (retval == PTHREAD_CANCELED);
> +  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "r_to_rc got to fclose");
> +
> +  free (temp_dir);
> +  free (file1);
> +  free (file2);
> +  free (file3);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/stdio-common/tst-freopen7.c b/stdio-common/tst-freopen7.c
> new file mode 100644
> index 0000000000..03d0de798e
> --- /dev/null
> +++ b/stdio-common/tst-freopen7.c
> @@ -0,0 +1,2 @@
> +#define FREOPEN freopen
> +#include <tst-freopen7-main.c>
> 
>
H.J. Lu Oct. 8, 2024, 12:54 a.m. UTC | #3
On Tue, Oct 1, 2024 at 9:00 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 12/09/24 19:43, Joseph Myers wrote:
> > On Thu, 12 Sep 2024, Florian Weimer wrote:
> >
> >> * Joseph Myers:
> >>
> >>> Note that it's in the nature of the uncertain time at which
> >>> cancellation might act (possibly during freopen, possibly during
> >>> subsequent reads) that these can leak memory or file descriptors, so
> >>> these do not include leak tests.
> >>
> >> Hmm.  Is it even possible for applications to handle this correctly?
> >> Given that freopen closes the original stream on error?  So the
> >> cancellation handler can't know if the stream is still open or not.
> >
> > I don't expect this to work particularly well in applications (in terms of
> > knowing if the stream is open).
> >
> >>> +ifeq ($(subdir),stdio-common)
> >>> +tests += \
> >>> +  tst-freopen-cancel \
> >>> +  tst-freopen64-cancel \
> >>> +  # tests
> >>> +
> >>> +$(objpfx)tst-freopen-cancel: $(shared-thread-library)
> >>> +$(objpfx)tst-freopen64-cancel: $(shared-thread-library)
> >>> +endif
> >>
> >> The test could be in stdio-common.  It doesn't need serial execution.
> >
> > I've moved the test.  This makes it depend on my previous patch with other
> > special-case tests (textually in the Makefile, not in any substantive
> > way).
> >
> >>> +  fp2 = xfopen (file3, "wc");
> >>> +  fputs ("rc_to_r got to freopen", fp2);
> >>> +  xfclose (fp2);
> >>> +  /* Cancellation should occur at some point from here onwards
> >>> +     (possibly leaking memory and file descriptors associated with the
> >>> +     FILE).  */
> >>> +  fp = FREOPEN (file2, "r", fp);
> >>> +  TEST_VERIFY_EXIT (fp != NULL);
> >>> +  for (;;)
> >>> +    {
> >>> +      fgetc (fp);
> >>> +      fseek (fp, 0, SEEK_SET);
> >>> +    }
> >>> +}
> >>
> >> The test does not assert that cancellation happens in the expected
> >> region.  You could set up fp in the main thread and pass it via the
> >> test_rc_to_r pointer argument?
> >
> > It makes sure cancellation only occurs after the checked-for text is
> > written to file3.
> >
> >>> +void *
> >>> +test_r_to_rc (void *p)
> >>> +{
> >>> +  int ret;
> >>> +  FILE *fp;
> >>> +  fp = xfopen (file1, "r");
> >>> +  fp = FREOPEN (file2, "rc", fp);
> >>> +  TEST_VERIFY_EXIT (fp != NULL);
> >>> +  ret = sem_post (&sem);
> >>> +  TEST_VERIFY_EXIT (ret == 0);
> >>> +  /* No cancellation should occur for I/O on file2.  */
> >>> +  for (int i = 0; i < 1000000; i++)
> >>> +    {
> >>> +      fgetc (fp);
> >>> +      fseek (fp, 0, SEEK_SET);
> >>> +    }
> >>
> >> Maybe you could use a FIFO and <support/process_state.h> to avoid
> >> spinning here?  Call pthread_cancel only after
> >> support_process_state_wait reports reaching
> >> support_process_state_sleeping?
> >
> > I'm not sure a check for sleeping is right, especially since we're
> > concerned about the state of a thread not a process.  But I've changed to
> > use a fifo and avoided spinning that way.
> >
> >>> +  xfclose (fp);
> >>> +  fp = xfopen (file3, "wc");
> >>> +  fputs ("r_to_rc got to fclose", fp);
> >>> +  xfclose (fp);
> >>> +  for (;;)
> >>> +    pthread_testcancel ();
> >>> +}
> >>
> >> I assume you wrote it this way to assert that there actually is a
> >> pending cancellation request.  I suggest to set a global variable after
> >> the xfclose to make sure that the cancellation was not acted upon in
> >> fclose.
> >
> > If cancellation occurred in the first xfclose, then the check for file3
> > contents would fail.  There's an implicit assumption that "c" works OK
> > with fopen so the second xfclose isn't an issue (just changes in freopen
> > are being tested here).
> >
> >
> > Add freopen special-case tests: thread cancellation
> >
> > Add tests of freopen adding or removing "c" (non-cancelling I/O) from
> > the mode string (so completing my planned tests of freopen with
> > different features used in the mode strings).  Note that it's in the
> > nature of the uncertain time at which cancellation might act (possibly
> > during freopen, possibly during subsequent reads) that these can leak
> > memory or file descriptors, so these do not include leak tests.
> >
> > Tested for x86_64.
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> >
> > ---
> >
> > Changed in v2: moved tests to stdio-common; use a fifo in test_r_to_rc
> > to avoid any need to spin reading or calling pthread_testcancel.
> >
> > diff --git a/stdio-common/Makefile b/stdio-common/Makefile
> > index 62f8b99b06..79da56d055 100644
> > --- a/stdio-common/Makefile
> > +++ b/stdio-common/Makefile
> > @@ -222,10 +222,12 @@ tests := \
> >    tst-freopen4 \
> >    tst-freopen5 \
> >    tst-freopen6 \
> > +  tst-freopen7 \
> >    tst-freopen64-2 \
> >    tst-freopen64-3 \
> >    tst-freopen64-4 \
> >    tst-freopen64-6 \
> > +  tst-freopen64-7 \
> >    tst-fseek \
> >    tst-fwrite \
> >    tst-fwrite-memstrm \
> > @@ -620,3 +622,6 @@ $(objpfx)tst-setvbuf1-cmp.out: tst-setvbuf1.expect $(objpfx)tst-setvbuf1.out
> >
> >  $(objpfx)tst-printf-round: $(libm)
> >  $(objpfx)tst-scanf-round: $(libm)
> > +
> > +$(objpfx)tst-freopen7: $(shared-thread-library)
> > +$(objpfx)tst-freopen64-7: $(shared-thread-library)
> > diff --git a/stdio-common/tst-freopen64-7.c b/stdio-common/tst-freopen64-7.c
> > new file mode 100644
> > index 0000000000..f34c280521
> > --- /dev/null
> > +++ b/stdio-common/tst-freopen64-7.c
> > @@ -0,0 +1,2 @@
> > +#define FREOPEN freopen64
> > +#include <tst-freopen7-main.c>
> > diff --git a/stdio-common/tst-freopen7-main.c b/stdio-common/tst-freopen7-main.c
> > new file mode 100644
> > index 0000000000..965e0b4adc
> > --- /dev/null
> > +++ b/stdio-common/tst-freopen7-main.c
> > @@ -0,0 +1,155 @@
> > +/* Test freopen cancellation handling.
> > +   Copyright (C) 2024 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <mcheck.h>
> > +#include <pthread.h>
> > +#include <semaphore.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <wchar.h>
> > +
> > +#include <support/check.h>
> > +#include <support/file_contents.h>
> > +#include <support/support.h>
> > +#include <support/temp_file.h>
> > +#include <support/test-driver.h>
> > +#include <support/xstdio.h>
> > +#include <support/xthread.h>
> > +#include <support/xunistd.h>
> > +
> > +char *file1, *file2, *file3, *fifo;
> > +
> > +sem_t sem;
> > +
> > +void *
> > +test_rc_to_r (void *p)
> > +{
> > +  int ret;
> > +  FILE *fp, *fp2;
> > +  ret = sem_post (&sem);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  fp = xfopen (file1, "rc");
> > +  for (int i = 0; i < 1000000; i++)
> > +    {
> > +      fgetc (fp);
> > +      fseek (fp, 0, SEEK_SET);
> > +    }
> > +  fp2 = xfopen (file3, "wc");
> > +  fputs ("rc_to_r got to freopen", fp2);
> > +  xfclose (fp2);
> > +  /* Cancellation should occur at some point from here onwards
> > +     (possibly leaking memory and file descriptors associated with the
> > +     FILE).  */
> > +  fp = FREOPEN (file2, "r", fp);
> > +  TEST_VERIFY_EXIT (fp != NULL);
> > +  for (;;)
> > +    {
> > +      fgetc (fp);
> > +      fseek (fp, 0, SEEK_SET);
> > +    }
> > +}
> > +
> > +void *
> > +test_r_to_rc (void *p)
> > +{
> > +  int ret;
> > +  FILE *fp;
> > +  fp = xfopen (file1, "r");
> > +  fp = FREOPEN (fifo, "rc", fp);
> > +  TEST_VERIFY_EXIT (fp != NULL);
> > +  ret = sem_post (&sem);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  /* No cancellation should occur for I/O on fifo.  */
> > +  ret = fgetc (fp);
> > +  /* At this point, the other thread has called pthread_cancel and
> > +     then written a byte to the fifo, so this thread is cancelled at
> > +     the next cancellation point.  */
> > +  TEST_VERIFY (ret == 'x');
> > +  xfclose (fp);
> > +  fp = xfopen (file3, "wc");
> > +  fputs ("r_to_rc got to fclose", fp);
> > +  xfclose (fp);
> > +  pthread_testcancel ();
> > +  FAIL_EXIT1 ("test_r_to_rc not cancelled\n");
> > +}
> > +
> > +int
> > +do_test (void)
> > +{
> > +  char *temp_dir = support_create_temp_directory ("tst-freopen-cancel");
> > +  file1 = xasprintf ("%s/file1", temp_dir);
> > +  support_write_file_string (file1, "file1");
> > +  add_temp_file (file1);
> > +  file2 = xasprintf ("%s/file2", temp_dir);
> > +  support_write_file_string (file2, "file2");
> > +  add_temp_file (file2);
> > +  file3 = xasprintf ("%s/file3", temp_dir);
> > +  support_write_file_string (file3, "file3");
> > +  add_temp_file (file3);
> > +  fifo = xasprintf ("%s/fifo", temp_dir);
> > +  xmkfifo (fifo, 0666);
> > +  add_temp_file (fifo);
> > +  int ret;
> > +  pthread_t thr;
> > +  void *retval;
> > +
> > +  /* Test changing to/from c (cancellation disabled).  */
> > +
> > +  verbose_printf ("Testing rc -> r\n");
> > +  ret = sem_init (&sem, 0, 0);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  thr = xpthread_create (NULL, test_rc_to_r, NULL);
> > +  ret = sem_wait (&sem);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  xpthread_cancel (thr);
> > +  ret = pthread_join (thr, &retval);
> > +  TEST_COMPARE (ret, 0);
> > +  TEST_VERIFY (retval == PTHREAD_CANCELED);
> > +  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "rc_to_r got to freopen");
> > +
> > +  verbose_printf ("Testing r -> rc\n");
> > +  ret = sem_init (&sem, 0, 0);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  thr = xpthread_create (NULL, test_r_to_rc, NULL);
> > +  FILE *fp = xfopen (fifo, "w");
> > +  ret = sem_wait (&sem);
> > +  TEST_VERIFY_EXIT (ret == 0);
> > +  /* This call happens while, or before, the other thread is waiting
> > +     to read a character from the fifo.  It thus verifies that
> > +     cancellation does not occur from the fgetc call in that thread
> > +     (it should instead occur only in pthread_testcancel call),
> > +     because the expected string is only written to file3 after that
> > +     thread closes the fifo.  */
> > +  xpthread_cancel (thr);
> > +  fputc ('x', fp);
> > +  xfclose (fp);
> > +  ret = pthread_join (thr, &retval);
> > +  TEST_COMPARE (ret, 0);
> > +  TEST_VERIFY (retval == PTHREAD_CANCELED);
> > +  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "r_to_rc got to fclose");
> > +
> > +  free (temp_dir);
> > +  free (file1);
> > +  free (file2);
> > +  free (file3);
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>
> > diff --git a/stdio-common/tst-freopen7.c b/stdio-common/tst-freopen7.c
> > new file mode 100644
> > index 0000000000..03d0de798e
> > --- /dev/null
> > +++ b/stdio-common/tst-freopen7.c
> > @@ -0,0 +1,2 @@
> > +#define FREOPEN freopen
> > +#include <tst-freopen7-main.c>
> >
> >
>

I am checking in this.
diff mbox series

Patch

diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index 62f8b99b06..79da56d055 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -222,10 +222,12 @@  tests := \
   tst-freopen4 \
   tst-freopen5 \
   tst-freopen6 \
+  tst-freopen7 \
   tst-freopen64-2 \
   tst-freopen64-3 \
   tst-freopen64-4 \
   tst-freopen64-6 \
+  tst-freopen64-7 \
   tst-fseek \
   tst-fwrite \
   tst-fwrite-memstrm \
@@ -620,3 +622,6 @@  $(objpfx)tst-setvbuf1-cmp.out: tst-setvbuf1.expect $(objpfx)tst-setvbuf1.out
 
 $(objpfx)tst-printf-round: $(libm)
 $(objpfx)tst-scanf-round: $(libm)
+
+$(objpfx)tst-freopen7: $(shared-thread-library)
+$(objpfx)tst-freopen64-7: $(shared-thread-library)
diff --git a/stdio-common/tst-freopen64-7.c b/stdio-common/tst-freopen64-7.c
new file mode 100644
index 0000000000..f34c280521
--- /dev/null
+++ b/stdio-common/tst-freopen64-7.c
@@ -0,0 +1,2 @@ 
+#define FREOPEN freopen64
+#include <tst-freopen7-main.c>
diff --git a/stdio-common/tst-freopen7-main.c b/stdio-common/tst-freopen7-main.c
new file mode 100644
index 0000000000..965e0b4adc
--- /dev/null
+++ b/stdio-common/tst-freopen7-main.c
@@ -0,0 +1,155 @@ 
+/* Test freopen cancellation handling.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <mcheck.h>
+#include <pthread.h>
+#include <semaphore.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <wchar.h>
+
+#include <support/check.h>
+#include <support/file_contents.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xstdio.h>
+#include <support/xthread.h>
+#include <support/xunistd.h>
+
+char *file1, *file2, *file3, *fifo;
+
+sem_t sem;
+
+void *
+test_rc_to_r (void *p)
+{
+  int ret;
+  FILE *fp, *fp2;
+  ret = sem_post (&sem);
+  TEST_VERIFY_EXIT (ret == 0);
+  fp = xfopen (file1, "rc");
+  for (int i = 0; i < 1000000; i++)
+    {
+      fgetc (fp);
+      fseek (fp, 0, SEEK_SET);
+    }
+  fp2 = xfopen (file3, "wc");
+  fputs ("rc_to_r got to freopen", fp2);
+  xfclose (fp2);
+  /* Cancellation should occur at some point from here onwards
+     (possibly leaking memory and file descriptors associated with the
+     FILE).  */
+  fp = FREOPEN (file2, "r", fp);
+  TEST_VERIFY_EXIT (fp != NULL);
+  for (;;)
+    {
+      fgetc (fp);
+      fseek (fp, 0, SEEK_SET);
+    }
+}
+
+void *
+test_r_to_rc (void *p)
+{
+  int ret;
+  FILE *fp;
+  fp = xfopen (file1, "r");
+  fp = FREOPEN (fifo, "rc", fp);
+  TEST_VERIFY_EXIT (fp != NULL);
+  ret = sem_post (&sem);
+  TEST_VERIFY_EXIT (ret == 0);
+  /* No cancellation should occur for I/O on fifo.  */
+  ret = fgetc (fp);
+  /* At this point, the other thread has called pthread_cancel and
+     then written a byte to the fifo, so this thread is cancelled at
+     the next cancellation point.  */
+  TEST_VERIFY (ret == 'x');
+  xfclose (fp);
+  fp = xfopen (file3, "wc");
+  fputs ("r_to_rc got to fclose", fp);
+  xfclose (fp);
+  pthread_testcancel ();
+  FAIL_EXIT1 ("test_r_to_rc not cancelled\n");
+}
+
+int
+do_test (void)
+{
+  char *temp_dir = support_create_temp_directory ("tst-freopen-cancel");
+  file1 = xasprintf ("%s/file1", temp_dir);
+  support_write_file_string (file1, "file1");
+  add_temp_file (file1);
+  file2 = xasprintf ("%s/file2", temp_dir);
+  support_write_file_string (file2, "file2");
+  add_temp_file (file2);
+  file3 = xasprintf ("%s/file3", temp_dir);
+  support_write_file_string (file3, "file3");
+  add_temp_file (file3);
+  fifo = xasprintf ("%s/fifo", temp_dir);
+  xmkfifo (fifo, 0666);
+  add_temp_file (fifo);
+  int ret;
+  pthread_t thr;
+  void *retval;
+
+  /* Test changing to/from c (cancellation disabled).  */
+
+  verbose_printf ("Testing rc -> r\n");
+  ret = sem_init (&sem, 0, 0);
+  TEST_VERIFY_EXIT (ret == 0);
+  thr = xpthread_create (NULL, test_rc_to_r, NULL);
+  ret = sem_wait (&sem);
+  TEST_VERIFY_EXIT (ret == 0);
+  xpthread_cancel (thr);
+  ret = pthread_join (thr, &retval);
+  TEST_COMPARE (ret, 0);
+  TEST_VERIFY (retval == PTHREAD_CANCELED);
+  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "rc_to_r got to freopen");
+
+  verbose_printf ("Testing r -> rc\n");
+  ret = sem_init (&sem, 0, 0);
+  TEST_VERIFY_EXIT (ret == 0);
+  thr = xpthread_create (NULL, test_r_to_rc, NULL);
+  FILE *fp = xfopen (fifo, "w");
+  ret = sem_wait (&sem);
+  TEST_VERIFY_EXIT (ret == 0);
+  /* This call happens while, or before, the other thread is waiting
+     to read a character from the fifo.  It thus verifies that
+     cancellation does not occur from the fgetc call in that thread
+     (it should instead occur only in pthread_testcancel call),
+     because the expected string is only written to file3 after that
+     thread closes the fifo.  */
+  xpthread_cancel (thr);
+  fputc ('x', fp);
+  xfclose (fp);
+  ret = pthread_join (thr, &retval);
+  TEST_COMPARE (ret, 0);
+  TEST_VERIFY (retval == PTHREAD_CANCELED);
+  TEST_OPEN_AND_COMPARE_FILE_STRING (file3, "r_to_rc got to fclose");
+
+  free (temp_dir);
+  free (file1);
+  free (file2);
+  free (file3);
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/stdio-common/tst-freopen7.c b/stdio-common/tst-freopen7.c
new file mode 100644
index 0000000000..03d0de798e
--- /dev/null
+++ b/stdio-common/tst-freopen7.c
@@ -0,0 +1,2 @@ 
+#define FREOPEN freopen
+#include <tst-freopen7-main.c>