[incubator-nuttx] branch master updated: sched/pthread/pthread_condwait.c: Fix cancellation bug.

[ag...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

aguettouche pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 64b398f  sched/pthread/pthread_condwait.c:  Fix cancellation bug.
64b398f is described below

commit 64b398f4c6e0e3aeebac9423ccb40395a8c35aea
Author: Gregory Nutt <gn...@nuttx.org>
AuthorDate: Sun Apr 5 14:52:23 2020 -0600

    sched/pthread/pthread_condwait.c:  Fix cancellation bug.
    
    Now that nxsem_wait_uninterruptible() returns an error when the thread is canceled, new logic is being executed.  This revealed an existing error in pthread_cond_wait().  The nature of the error is as follows:
    
    * pthread_cond_wait() must atomically (1) release the mutex, (2) wait on the condition, and (3) re-establish the mutex.  Errors can occur on any of these steps.  In this case the ECANCELED error was (very correctly) reported by nxsem_wait_uninterruptible().
    
    * However, logic in step (3) had a bug; it re-acquired the mutex, but then a bug in existing logic cause the mutex to be improperly initialized.  Essentially, the wrong error status value was being testing.  This resulted in a nonsequitar and errors reported by the OS test.
    
    This problem was found by apps/testing/ostest/pthread_cleanup.c
---
 sched/pthread/pthread_condwait.c | 54 +++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/sched/pthread/pthread_condwait.c b/sched/pthread/pthread_condwait.c
index 7981211..cc866df 100644
--- a/sched/pthread/pthread_condwait.c
+++ b/sched/pthread/pthread_condwait.c
@@ -1,35 +1,20 @@
 /****************************************************************************
  * sched/pthread/pthread_condwait.c
  *
- *   Copyright (C) 2007-2009, 2012, 2016 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <gn...@nuttx.org>
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
  *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
+ *   http://www.apache.org/licenses/LICENSE-2.0
  *
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- * 3. Neither the name NuttX nor the names of its contributors may be
- *    used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
- * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
  *
  ****************************************************************************/
 
@@ -101,9 +86,11 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
 
       sched_lock();
       mutex->pid = -1;
-      ret = pthread_mutex_give(mutex);
+      ret        = pthread_mutex_give(mutex);
 
-      /* Take the semaphore */
+      /* Take the semaphore.  This may be awakened only be a signal (EINTR)
+       * or if the thread is canceled (ECANCELED)
+       */
 
       status = pthread_sem_take((FAR sem_t *)&cond->sem, NULL, false);
       if (ret == OK)
@@ -132,11 +119,16 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex)
           ret = status;
         }
 
-      /* Was all of the above successful? */
+      /* Did we get the mutex? */
 
-      if (ret == OK)
+      if (status == OK)
         {
+          /* Yes.. Then initialize it properly */
+
           mutex->pid = getpid();
+#ifdef CONFIG_PTHREAD_MUTEX_TYPES
+          mutex->nlocks = 1;
+#endif
         }
     }