[SSSD] [sssd PR#46][synchronized] sss_client: Defer thread cancellation until completion of nss/pam operations

2016-11-24 Thread HouzuoGuo 
   URL: https://github.com/SSSD/sssd/pull/46
Author: HouzuoGuo
 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam 
operations
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/46/head:pr46
git checkout pr46
From 3eae3802347e244a032624733ae28b720feba1d5 Mon Sep 17 00:00:00 2001
From: Howard Guo 
Date: Tue, 11 Oct 2016 10:35:13 +0200
Subject: [PATCH] sss_client: Defer thread cancellation until completion of
 nss/pam operations

https://fedorahosted.org/sssd/ticket/3156

The client code is not cancellation-safe, an application which has cancelled an NSS operation
will experience subtle bugs, hence thread cancellation is deferred until completion of client
operations.
---
 Makefile.am |  4 ---
 src/sss_client/common.c | 80 +
 2 files changed, 7 insertions(+), 77 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1ea8f50..ca561de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -771,10 +771,6 @@ endif
 
 CLIENT_LIBS = $(LTLIBINTL)
 
-if HAVE_PTHREAD
-CLIENT_LIBS += -lpthread
-endif
-
 if WITH_JOURNALD
 SYSLOG_LIBS = $(JOURNALD_LIBS)
 endif
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 20106b1..b7a5ed7 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1070,86 +1070,28 @@ typedef void (*sss_mutex_init)(void);
 struct sss_mutex {
 pthread_mutex_t mtx;
 
-pthread_once_t once;
-sss_mutex_init init;
+int old_cancel_state;
 };
 
-static void sss_nss_mt_init(void);
-static void sss_pam_mt_init(void);
-static void sss_nss_mc_mt_init(void);
+static struct sss_mutex sss_nss_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER };
 
-static struct sss_mutex sss_nss_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_nss_mt_init };
+static struct sss_mutex sss_pam_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER };
 
-static struct sss_mutex sss_pam_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_pam_mt_init };
-
-static struct sss_mutex sss_nss_mc_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-   .once = PTHREAD_ONCE_INIT,
-   .init = sss_nss_mc_mt_init };
-
-/* Wrappers for robust mutex support */
-static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
-{
-#ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST
-return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST);
-#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP)
-return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-static int sss_mutex_consistent(pthread_mutex_t *mtx)
-{
-#ifdef HAVE_PTHREAD_MUTEX_CONSISTENT
-return pthread_mutex_consistent(mtx);
-#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP)
-return pthread_mutex_consistent_np(mtx);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-/* Generic mutex init, lock, unlock functions */
-static void sss_mt_init(struct sss_mutex *m)
-{
-pthread_mutexattr_t attr;
-
-if (pthread_mutexattr_init() != 0) {
-return;
-}
-if (sss_mutexattr_setrobust() != 0) {
-return;
-}
-
-pthread_mutex_init(>mtx, );
-pthread_mutexattr_destroy();
-}
+static struct sss_mutex sss_nss_mc_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER };
 
 static void sss_mt_lock(struct sss_mutex *m)
 {
-pthread_once(>once, m->init);
-if (pthread_mutex_lock(>mtx) == EOWNERDEAD) {
-sss_cli_close_socket();
-sss_mutex_consistent(>mtx);
-}
+pthread_mutex_lock(>mtx);
+pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, >old_cancel_state);
 }
 
 static void sss_mt_unlock(struct sss_mutex *m)
 {
+pthread_setcancelstate(m->old_cancel_state, NULL);
 pthread_mutex_unlock(>mtx);
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mt_init(void)
-{
-sss_mt_init(_nss_mtx);
-}
 void sss_nss_lock(void)
 {
 sss_mt_lock(_nss_mtx);
@@ -1160,10 +1102,6 @@ void sss_nss_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_pam_mt_init(void)
-{
-sss_mt_init(_pam_mtx);
-}
 void sss_pam_lock(void)
 {
 sss_mt_lock(_pam_mtx);
@@ -1174,10 +1112,6 @@ void sss_pam_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mc_mt_init(void)
-{
-sss_mt_init(_nss_mc_mtx);
-}
 void sss_nss_mc_lock(void)
 {
 sss_mt_lock(_nss_mc_mtx);
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#46][synchronized] sss_client: Defer thread cancellation until completion of nss/pam operations

2016-11-24 Thread HouzuoGuo 
   URL: https://github.com/SSSD/sssd/pull/46
Author: HouzuoGuo
 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam 
operations
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/46/head:pr46
git checkout pr46
From 176a84c1add79398f342157a12945838bbb1ef0e Mon Sep 17 00:00:00 2001
From: Howard Guo 
Date: Tue, 11 Oct 2016 10:35:13 +0200
Subject: [PATCH 1/3] sss_client: Defer thread cancellation until completion of
 nss/pam operations

https://fedorahosted.org/sssd/ticket/3156

The client code is not cancellation-safe, an application which has cancelled an NSS operation
will experience subtle bugs, hence thread cancellation is deferred until completion of client
operations.
---
 Makefile.am |  4 ---
 src/sss_client/common.c | 74 +
 2 files changed, 7 insertions(+), 71 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1ea8f50..ca561de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -771,10 +771,6 @@ endif
 
 CLIENT_LIBS = $(LTLIBINTL)
 
-if HAVE_PTHREAD
-CLIENT_LIBS += -lpthread
-endif
-
 if WITH_JOURNALD
 SYSLOG_LIBS = $(JOURNALD_LIBS)
 endif
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 20106b1..61ec55e 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1071,85 +1071,33 @@ struct sss_mutex {
 pthread_mutex_t mtx;
 
 pthread_once_t once;
-sss_mutex_init init;
+int old_cancel_state;
 };
 
-static void sss_nss_mt_init(void);
-static void sss_pam_mt_init(void);
 static void sss_nss_mc_mt_init(void);
 
 static struct sss_mutex sss_nss_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_nss_mt_init };
+.once = PTHREAD_ONCE_INIT };
 
 static struct sss_mutex sss_pam_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_pam_mt_init };
+.once = PTHREAD_ONCE_INIT };
 
 static struct sss_mutex sss_nss_mc_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-   .once = PTHREAD_ONCE_INIT,
-   .init = sss_nss_mc_mt_init };
-
-/* Wrappers for robust mutex support */
-static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
-{
-#ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST
-return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST);
-#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP)
-return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-static int sss_mutex_consistent(pthread_mutex_t *mtx)
-{
-#ifdef HAVE_PTHREAD_MUTEX_CONSISTENT
-return pthread_mutex_consistent(mtx);
-#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP)
-return pthread_mutex_consistent_np(mtx);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-/* Generic mutex init, lock, unlock functions */
-static void sss_mt_init(struct sss_mutex *m)
-{
-pthread_mutexattr_t attr;
-
-if (pthread_mutexattr_init() != 0) {
-return;
-}
-if (sss_mutexattr_setrobust() != 0) {
-return;
-}
-
-pthread_mutex_init(>mtx, );
-pthread_mutexattr_destroy();
-}
+   .once = PTHREAD_ONCE_INIT };
 
 static void sss_mt_lock(struct sss_mutex *m)
 {
-pthread_once(>once, m->init);
-if (pthread_mutex_lock(>mtx) == EOWNERDEAD) {
-sss_cli_close_socket();
-sss_mutex_consistent(>mtx);
-}
+pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, >old_cancel_state);
+pthread_mutex_lock(>mtx);
 }
 
 static void sss_mt_unlock(struct sss_mutex *m)
 {
 pthread_mutex_unlock(>mtx);
+pthread_setcancelstate(m->old_cancel_state, NULL);
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mt_init(void)
-{
-sss_mt_init(_nss_mtx);
-}
 void sss_nss_lock(void)
 {
 sss_mt_lock(_nss_mtx);
@@ -1160,10 +1108,6 @@ void sss_nss_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_pam_mt_init(void)
-{
-sss_mt_init(_pam_mtx);
-}
 void sss_pam_lock(void)
 {
 sss_mt_lock(_pam_mtx);
@@ -1174,10 +1118,6 @@ void sss_pam_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mc_mt_init(void)
-{
-sss_mt_init(_nss_mc_mtx);
-}
 void sss_nss_mc_lock(void)
 {
 sss_mt_lock(_nss_mc_mtx);

From 22762ebecdead93764197b0fc245a1d6909b68d7 Mon Sep 17 00:00:00 2001
From: HouzuoGuo 
Date: Wed, 9 Nov 2016 12:10:25 +0100
Subject: [PATCH 2/3] Remove unused sss_mutex.once; correct the order of
 locking mutex and setting thread cancel state.

---
 src/sss_client/common.c | 14 +-
 1 file

[SSSD] [sssd PR#46][synchronized] sss_client: Defer thread cancellation until completion of nss/pam operations

2016-11-09 Thread HouzuoGuo 
   URL: https://github.com/SSSD/sssd/pull/46
Author: HouzuoGuo
 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam 
operations
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/46/head:pr46
git checkout pr46
From 176a84c1add79398f342157a12945838bbb1ef0e Mon Sep 17 00:00:00 2001
From: Howard Guo 
Date: Tue, 11 Oct 2016 10:35:13 +0200
Subject: [PATCH 1/2] sss_client: Defer thread cancellation until completion of
 nss/pam operations

https://fedorahosted.org/sssd/ticket/3156

The client code is not cancellation-safe, an application which has cancelled an NSS operation
will experience subtle bugs, hence thread cancellation is deferred until completion of client
operations.
---
 Makefile.am |  4 ---
 src/sss_client/common.c | 74 +
 2 files changed, 7 insertions(+), 71 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1ea8f50..ca561de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -771,10 +771,6 @@ endif
 
 CLIENT_LIBS = $(LTLIBINTL)
 
-if HAVE_PTHREAD
-CLIENT_LIBS += -lpthread
-endif
-
 if WITH_JOURNALD
 SYSLOG_LIBS = $(JOURNALD_LIBS)
 endif
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 20106b1..61ec55e 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1071,85 +1071,33 @@ struct sss_mutex {
 pthread_mutex_t mtx;
 
 pthread_once_t once;
-sss_mutex_init init;
+int old_cancel_state;
 };
 
-static void sss_nss_mt_init(void);
-static void sss_pam_mt_init(void);
 static void sss_nss_mc_mt_init(void);
 
 static struct sss_mutex sss_nss_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_nss_mt_init };
+.once = PTHREAD_ONCE_INIT };
 
 static struct sss_mutex sss_pam_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_pam_mt_init };
+.once = PTHREAD_ONCE_INIT };
 
 static struct sss_mutex sss_nss_mc_mtx = { .mtx  = PTHREAD_MUTEX_INITIALIZER,
-   .once = PTHREAD_ONCE_INIT,
-   .init = sss_nss_mc_mt_init };
-
-/* Wrappers for robust mutex support */
-static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
-{
-#ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST
-return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST);
-#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP)
-return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-static int sss_mutex_consistent(pthread_mutex_t *mtx)
-{
-#ifdef HAVE_PTHREAD_MUTEX_CONSISTENT
-return pthread_mutex_consistent(mtx);
-#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP)
-return pthread_mutex_consistent_np(mtx);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-/* Generic mutex init, lock, unlock functions */
-static void sss_mt_init(struct sss_mutex *m)
-{
-pthread_mutexattr_t attr;
-
-if (pthread_mutexattr_init() != 0) {
-return;
-}
-if (sss_mutexattr_setrobust() != 0) {
-return;
-}
-
-pthread_mutex_init(>mtx, );
-pthread_mutexattr_destroy();
-}
+   .once = PTHREAD_ONCE_INIT };
 
 static void sss_mt_lock(struct sss_mutex *m)
 {
-pthread_once(>once, m->init);
-if (pthread_mutex_lock(>mtx) == EOWNERDEAD) {
-sss_cli_close_socket();
-sss_mutex_consistent(>mtx);
-}
+pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, >old_cancel_state);
+pthread_mutex_lock(>mtx);
 }
 
 static void sss_mt_unlock(struct sss_mutex *m)
 {
 pthread_mutex_unlock(>mtx);
+pthread_setcancelstate(m->old_cancel_state, NULL);
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mt_init(void)
-{
-sss_mt_init(_nss_mtx);
-}
 void sss_nss_lock(void)
 {
 sss_mt_lock(_nss_mtx);
@@ -1160,10 +1108,6 @@ void sss_nss_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_pam_mt_init(void)
-{
-sss_mt_init(_pam_mtx);
-}
 void sss_pam_lock(void)
 {
 sss_mt_lock(_pam_mtx);
@@ -1174,10 +1118,6 @@ void sss_pam_unlock(void)
 }
 
 /* NSS mutex wrappers */
-static void sss_nss_mc_mt_init(void)
-{
-sss_mt_init(_nss_mc_mtx);
-}
 void sss_nss_mc_lock(void)
 {
 sss_mt_lock(_nss_mc_mtx);

From 22762ebecdead93764197b0fc245a1d6909b68d7 Mon Sep 17 00:00:00 2001
From: HouzuoGuo 
Date: Wed, 9 Nov 2016 12:10:25 +0100
Subject: [PATCH 2/2] Remove unused sss_mutex.once; correct the order of
 locking mutex and setting thread cancel state.

---
 src/sss_client/common.c | 14 +-
 1 file