Re: [PATCH] system-cancel part2

[Christopher Faylor]

On Wed, Jan 15, 2003 at 11:35:40AM -0500, Christopher Faylor wrote:
>>2003-01-15  Thomas Paff  <[EMAIL PROTECTED]>
>>
>>  * syscalls.cc (struct system_cleanup_args): New struct.
>>  (system_cleanup): New function.
>>  (system): Use pthread_cleanup_push and _pop to save and restore
>>  signal handlers and sigprocmask.
>
>Please do not check this in.  You are changing other parts of the code than the
>pthreads code and I want to study what you've done before you are approved to
>check this in.
>
>In other words, Robert's "as long as you have a test case" only applies to
>trivial changes or changes to pthread.cc, thread.cc, or thread.h.

Sorry for the long delay.  This patch looks ok to me.  Please feel free
to check it in along with your additional test cases.

cgf

Re: [PATCH] system-cancel part2

[Robert Collins]

On Thu, 2003-01-16 at 00:19, Thomas Pfaff wrote:
> On Wed, 15 Jan 2003, Robert Collins wrote:

> The test case was created to prove that system is a cancellation point
> even if the child process is already created and the system call is
> waiting on child termination.

Thank you for the new test cases. When Chris approves the system()
changes, I'm happy with the pthreads aspects of this.

Rob

-- 
GPG key available at: .



signature.asc
Description: This is a digitally signed message part

Re: [PATCH] system-cancel part2

[Christopher Faylor]

On Wed, Jan 15, 2003 at 12:23:14PM +0100, Thomas Pfaff wrote:
>
>This patch will make sure that the signal handlers that are saved in the
>system call are restored even if the thread got cancelled. Since
>spawn_guts uses waitpid when mode is _P_WAIT spawn_guts is a cancellation
>point.
>
>Attached is the patch and a new test case.
>
>2003-01-15  Thomas Paff  <[EMAIL PROTECTED]>
>
>   * syscalls.cc (struct system_cleanup_args): New struct.
>   (system_cleanup): New function.
>   (system): Use pthread_cleanup_push and _pop to save and restore
>   signal handlers and sigprocmask.

Please do not check this in.  You are changing other parts of the code than the
pthreads code and I want to study what you've done before you are approved to
check this in.

In other words, Robert's "as long as you have a test case" only applies to
trivial changes or changes to pthread.cc, thread.cc, or thread.h.

cgf

>diff -urp src.old/winsup/cygwin/syscalls.cc src/winsup/cygwin/syscalls.cc
>--- src.old/winsup/cygwin/syscalls.cc  2003-01-14 11:35:51.0 +0100
>+++ src/winsup/cygwin/syscalls.cc  2003-01-15 09:42:04.0 +0100
>@@ -1371,6 +1371,21 @@ done:
>   return res;
> }
> 
>+struct system_cleanup_args
>+{
>+  _sig_func_ptr oldint, oldquit;
>+  sigset_t old_mask;
>+};
>+
>+static void system_cleanup (void *args)
>+{
>+  struct system_cleanup_args *cleanup_args = (struct system_cleanup_args *) args;
>+
>+  signal (SIGINT, cleanup_args->oldint);
>+  signal (SIGQUIT, cleanup_args->oldquit);
>+  (void) sigprocmask (SIG_SETMASK, &cleanup_args->old_mask, 0);
>+}  
>+
> extern "C" int
> system (const char *cmdstring)
> {
>@@ -1382,23 +1397,25 @@ system (const char *cmdstring)
>   sigframe thisframe (mainthread);
>   int res;
>   const char* command[4];
>-  _sig_func_ptr oldint, oldquit;
>-  sigset_t child_block, old_mask;
>+  struct system_cleanup_args cleanup_args;
>+  sigset_t child_block;
> 
>   if (cmdstring == (const char *) NULL)
>   return 1;
> 
>-  oldint = signal (SIGINT, SIG_IGN);
>-  oldquit = signal (SIGQUIT, SIG_IGN);
>+  cleanup_args.oldint = signal (SIGINT, SIG_IGN);
>+  cleanup_args.oldquit = signal (SIGQUIT, SIG_IGN);
>   sigemptyset (&child_block);
>   sigaddset (&child_block, SIGCHLD);
>-  (void) sigprocmask (SIG_BLOCK, &child_block, &old_mask);
>+  (void) sigprocmask (SIG_BLOCK, &child_block, &cleanup_args.old_mask);
> 
>   command[0] = "sh";
>   command[1] = "-c";
>   command[2] = cmdstring;
>   command[3] = (const char *) NULL;
> 
>+  pthread_cleanup_push (system_cleanup, (void *) &cleanup_args);
>+
>   if ((res = spawnvp (_P_WAIT, "sh", command)) == -1)
> {
>   // when exec fails, return value should be as if shell
>@@ -1406,9 +1423,8 @@ system (const char *cmdstring)
>   res = 127;
> }
> 
>-  signal (SIGINT, oldint);
>-  signal (SIGQUIT, oldquit);
>-  (void) sigprocmask (SIG_SETMASK, &old_mask, 0);
>+  pthread_cleanup_pop (1);
>+
>   return res;
> }
> 

>/*
> * File: cancel11.c
> *
> * Test Synopsis: Test if system is a cancellation point.
> *
> * Test Method (Validation or Falsification):
> * - 
> *
> * Requirements Tested:
> * -
> *
> * Features Tested:
> * - 
> *
> * Cases Tested:
> * - 
> *
> * Description:
> * - 
> *
> * Environment:
> * - 
> *
> * Input:
> * - None.
> *
> * Output:
> * - File name, Line number, and failed expression on failure.
> * - No output on success.
> *
> * Assumptions:
> * - have working pthread_create, pthread_cancel, pthread_setcancelstate
> *   pthread_join
> *
> * Pass Criteria:
> * - Process returns zero exit status.
> *
> * Fail Criteria:
> * - Process returns non-zero exit status.
> */
>
>#include "test.h"
>
>static void *Thread(void *punused)
>{
>  system ("sleep 10");
>
>  return NULL;
>}
>
>int main (void)
>{
>  void * result;
>  pthread_t t;
>
>  assert (pthread_create (&t, NULL, Thread, NULL) == 0);
>  sleep (5);
>  assert (pthread_cancel (t) == 0);
>  assert (pthread_join (t, &result) == 0);
>  assert (result == PTHREAD_CANCELED);
>
>  return 0;
>}

Re: [PATCH] system-cancel part2

[Thomas Pfaff]

On Wed, 15 Jan 2003, Robert Collins wrote:

> On Wed, 2003-01-15 at 22:23, Thomas Pfaff wrote:
> > This patch will make sure that the signal handlers that are saved in the
> > system call are restored even if the thread got cancelled. Since
> > spawn_guts uses waitpid when mode is _P_WAIT spawn_guts is a cancellation
> > point.
> >
> > Attached is the patch and a new test case.
>
> The new test case doesn't appear to check that the signal handlers where
> saved. Am I misreading that?
>

The test case was created to prove that system is a cancellation point
even if the child process is already created and the system call is
waiting on child termination.

Atached are two test cases that will test if the signal handlers are
restored when the call get cancelled and has waited successfully for the
child.

Thomas

/*
 * File: cancel11.c
 *
 * Test Synopsis: Test if system is a cancellation point.
 *
 * Test Method (Validation or Falsification):
 * - 
 *
 * Requirements Tested:
 * -
 *
 * Features Tested:
 * - 
 *
 * Cases Tested:
 * - 
 *
 * Description:
 * - 
 *
 * Environment:
 * - 
 *
 * Input:
 * - None.
 *
 * Output:
 * - File name, Line number, and failed expression on failure.
 * - No output on success.
 *
 * Assumptions:
 * - have working pthread_create, pthread_cancel, pthread_setcancelstate
 *   pthread_join
 *
 * Pass Criteria:
 * - Process returns zero exit status.
 *
 * Fail Criteria:
 * - Process returns non-zero exit status.
 */

#include "test.h"

static void sig_handler(int sig)
{
}

static void *Thread(void *punused)
{
  system ("sleep 10");

  return NULL;
}

int main (void)
{
  void * result;
  pthread_t t;

  signal (SIGINT, sig_handler);

  assert (pthread_create (&t, NULL, Thread, NULL) == 0);
  sleep (5);
  assert (pthread_cancel (t) == 0);
  assert (pthread_join (t, &result) == 0);
  assert (result == PTHREAD_CANCELED);

  assert ((void *)signal (SIGINT, NULL) == sig_handler);

  return 0;
}

/*
 * File: cancel12.c
 *
 * Test Synopsis: Test if system is a cancellation point.
 *
 * Test Method (Validation or Falsification):
 * - 
 *
 * Requirements Tested:
 * -
 *
 * Features Tested:
 * - 
 *
 * Cases Tested:
 * - 
 *
 * Description:
 * - 
 *
 * Environment:
 * - 
 *
 * Input:
 * - None.
 *
 * Output:
 * - File name, Line number, and failed expression on failure.
 * - No output on success.
 *
 * Assumptions:
 * - have working pthread_create, pthread_cancel, pthread_setcancelstate
 *   pthread_join
 *
 * Pass Criteria:
 * - Process returns zero exit status.
 *
 * Fail Criteria:
 * - Process returns non-zero exit status.
 */

#include "test.h"

static void sig_handler(int sig)
{
}

static void *Thread(void *punused)
{
  signal (SIGINT, sig_handler);

  system ("sleep 5");

  assert ((void *)signal (SIGINT, NULL) == sig_handler);

  return NULL;
}

int main (void)
{
  void *old_sigh;
  void * result;
  pthread_t t;

  assert (pthread_create (&t, NULL, Thread, NULL) == 0);
  assert (pthread_join (t, &result) == 0);
  assert (result == NULL);

  return 0;
}

Re: [PATCH] system-cancel part2

[Robert Collins]

On Wed, 2003-01-15 at 22:23, Thomas Pfaff wrote:
> This patch will make sure that the signal handlers that are saved in the
> system call are restored even if the thread got cancelled. Since
> spawn_guts uses waitpid when mode is _P_WAIT spawn_guts is a cancellation
> point.
> 
> Attached is the patch and a new test case.

The new test case doesn't appear to check that the signal handlers where
saved. Am I misreading that?

Rob
-- 
GPG key available at: .



signature.asc
Description: This is a digitally signed message part