by NIIBE Yutaka
2018-11-26 03:10:46 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".
The branch, master has been updated
via 40c7923ea881881a48de8f303b0870ec5910e13d (commit)
via 9fb3f0f3f79e74166cce8e0781e97043f25890cc (commit)
via f45d6124696cc92ccdec09841b8182679c377997 (commit)
from fa1b1eaa4241ff3f0634c8bdf8591cbc7c464144 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 40c7923ea881881a48de8f303b0870ec5910e13d
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 12:07:36 2018 +0900
agent: Have a thread to wait for the child process of scdaemon.
* agent/call-scd.c (wait_child_thread): New.
(start_scd): Create a thread for wait_child_thread.
(agent_scd_check_aliveness): Remove.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/agent.h b/agent/agent.h
index 9baf596..8a3e5c6 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -546,7 +546,6 @@ int divert_writekey (ctrl_t ctrl, int force, const char *serialno,
void initialize_module_call_scd (void);
void agent_scd_dump_state (void);
int agent_scd_check_running (void);
-void agent_scd_check_aliveness (void);
int agent_reset_scd (ctrl_t ctrl);
int agent_card_learn (ctrl_t ctrl,
void (*kpinfo_cb)(void*, const char *),
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 592ddb9..c6d062a 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -207,6 +207,85 @@ atfork_cb (void *opaque, int where)
}
+static void *
+wait_child_thread (void *arg)
+{
+ int err;
+ struct scd_local_s *sl;
+
+#ifdef HAVE_W32_SYSTEM
+ HANDLE pid = (HANDLE)arg;
+
+ npth_unprotect ();
+ WaitForSingleObject ((HANDLE)pid, INFINITE);
+ npth_protect ();
+#else
+ int wstatus;
+ pid_t pid = (pid_t)(uintptr_t)arg;
+
+ again:
+ npth_unprotect ();
+ err = waitpid (pid, &wstatus, 0);
+ npth_protect ();
+
+ if (err < 0)
+ {
+ if (errno == EINTR)
+ goto again;
+ log_error ("waitpid failed: %s\n", strerror (errno));
+ return NULL;
+ }
+ else
+ {
+ if (WIFEXITED (wstatus))
+ log_info ("scdaemon finished (status %d)\n", WEXITSTATUS (wstatus));
+ else if (WIFSIGNALED (wstatus))
+ log_info ("scdaemon killed by signal %d\n", WTERMSIG (wstatus));
+ else
+ {
+ if (WIFSTOPPED (wstatus))
+ log_info ("scdaemon stopped by signal %d\n", WSTOPSIG (wstatus));
+ goto again;
+ }
+ }
+#endif
+
+ err = npth_mutex_lock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to acquire the start_scd lock: %s\n",
+ strerror (err));
+ }
+ else
+ {
+ assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
+
+ for (sl = scd_local_list; sl; sl = sl->next_local)
+ {
+ sl->invalid = 1;
+ if (!sl->in_use && sl->ctx)
+ {
+ assuan_release (sl->ctx);
+ sl->ctx = NULL;
+ }
+ }
+
+ primary_scd_ctx = NULL;
+ primary_scd_ctx_reusable = 0;
+
+ xfree (socket_name);
+ socket_name = NULL;
+
+ err = npth_mutex_unlock (&start_scd_lock);
+ if (err)
+ log_error ("failed to release the start_scd lock while"
+ " doing the aliveness check: %s\n", strerror (err));
+ }
+
+ return NULL;
+}
+
+
/* Fork off the SCdaemon if this has not already been done. Lock the
daemon and make sure that a proper context has been setup in CTRL.
This function might also lock the daemon, which means that the
@@ -427,6 +506,24 @@ start_scd (ctrl_t ctrl)
primary_scd_ctx = ctx;
primary_scd_ctx_reusable = 0;
+ {
+ npth_t thread;
+ npth_attr_t tattr;
+ pid_t pid;
+
+ pid = assuan_get_pid (primary_scd_ctx);
+ err = npth_attr_init (&tattr);
+ if (!err)
+ {
+ npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED);
+ err = npth_create (&thread, &tattr, wait_child_thread,
+ (void *)(uintptr_t)pid);
+ if (err)
+ log_error ("error spawning wait_child_thread: %s\n", strerror (err));
+ npth_attr_destroy (&tattr);
+ }
+ }
+
leave:
xfree (abs_homedir);
if (err)
@@ -456,91 +553,6 @@ agent_scd_check_running (void)
}
-/* Check whether the Scdaemon is still alive and clean it up if not. */
-void
-agent_scd_check_aliveness (void)
-{
- pid_t pid;
-#ifdef HAVE_W32_SYSTEM
- DWORD rc;
-#else
- int rc;
-#endif
- struct timespec abstime;
- int err;
-
- if (!primary_scd_ctx)
- return; /* No scdaemon running. */
-
- /* This is not a critical function so we use a short timeout while
- acquiring the lock. */
- npth_clock_gettime (&abstime);
- abstime.tv_sec += 1;
- err = npth_mutex_timedlock (&start_scd_lock, &abstime);
- if (err)
- {
- if (err == ETIMEDOUT)
- {
- if (opt.verbose > 1)
- log_info ("failed to acquire the start_scd lock while"
- " doing an aliveness check: %s\n", strerror (err));
- }
- else
- log_error ("failed to acquire the start_scd lock while"
- " doing an aliveness check: %s\n", strerror (err));
- return;
- }
-
- if (primary_scd_ctx)
- {
- pid = assuan_get_pid (primary_scd_ctx);
-#ifdef HAVE_W32_SYSTEM
- /* If we have a PID we disconnect if either GetExitProcessCode
- fails or if ir returns the exit code of the scdaemon. 259 is
- the error code for STILL_ALIVE. */
- if (pid != (pid_t)(void*)(-1) && pid
- && (!GetExitCodeProcess ((HANDLE)pid, &rc) || rc != 259))
-#else
- if (pid != (pid_t)(-1) && pid
- && ((rc=waitpid (pid, NULL, WNOHANG))==-1 || (rc == pid)) )
-#endif
- {
- /* Okay, scdaemon died. Disconnect the primary connection
- now but take care that it won't do another wait. Also
- cleanup all other connections and release their
- resources. The next use will start a new daemon then.
- Due to the use of the START_SCD_LOCK we are sure that
- none of these context are actually in use. */
- struct scd_local_s *sl;
-
- assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
-
- for (sl=scd_local_list; sl; sl = sl->next_local)
- {
- sl->invalid = 1;
- if (!sl->in_use && sl->ctx)
- {
- assuan_release (sl->ctx);
- sl->ctx = NULL;
- }
- }
-
- primary_scd_ctx = NULL;
- primary_scd_ctx_reusable = 0;
-
- xfree (socket_name);
- socket_name = NULL;
- }
- }
-
- err = npth_mutex_unlock (&start_scd_lock);
- if (err)
- log_error ("failed to release the start_scd lock while"
- " doing the aliveness check: %s\n", strerror (err));
-}
-
-
-
/* Reset the SCD if it has been used. Actually it is not a reset but
a cleanup of resources used by the current connection. */
int
diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
index 911064c..de6947d 100644
--- a/agent/gpg-agent.c
+++ b/agent/gpg-agent.c
@@ -2370,9 +2370,6 @@ handle_tick (void)
if (!last_minute)
last_minute = time (NULL);
- /* Check whether the scdaemon has died and cleanup in this case. */
- agent_scd_check_aliveness ();
-
/* If we are running as a child of another process, check whether
the parent is still alive and shutdown if not. */
#ifndef HAVE_W32_SYSTEM
commit 9fb3f0f3f79e74166cce8e0781e97043f25890cc
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 11:05:28 2018 +0900
agent: Defer calling assuan_release when it's still in use.
* agent/call-scd.c (struct scd_local_s): Remove LOCK, introduce IN_USE
and INVALID flags.
(unlock_scd): Call assuan_release when CTX is invalid.
(start_scd): Set IN_USE.
(agent_scd_check_aliveness): Don't call assuan_release when it's in use.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 954be02..592ddb9 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -54,11 +54,10 @@ struct scd_local_s
SCD_LOCAL_LIST (see below). */
struct scd_local_s *next_local;
- assuan_context_t ctx; /* NULL or session context for the SCdaemon
- used with this connection. */
- int locked; /* This flag is used to assert proper use of
- start_scd and unlock_scd. */
-
+ assuan_context_t ctx; /* NULL or session context for the SCdaemon
+ used with this connection. */
+ unsigned int in_use: 1; /* CTX is in use. */
+ unsigned int invalid:1; /* CTX is invalid, should be released. */
};
@@ -132,7 +131,7 @@ initialize_module_call_scd (void)
{
err = npth_mutex_init (&start_scd_lock, NULL);
if (err)
- log_fatal ("error initializing mutex: %s\n", strerror (err));
+ log_fatal ("error initializing mutex: %s\n", strerror (err));
initialized = 1;
}
}
@@ -162,14 +161,37 @@ agent_scd_dump_state (void)
static int
unlock_scd (ctrl_t ctrl, int rc)
{
- if (ctrl->scd_local->locked != 1)
+ int err;
+
+ if (ctrl->scd_local->in_use == 0)
{
- log_error ("unlock_scd: invalid lock count (%d)\n",
- ctrl->scd_local->locked);
+ log_error ("unlock_scd: CTX is not in use\n");
if (!rc)
rc = gpg_error (GPG_ERR_INTERNAL);
}
- ctrl->scd_local->locked = 0;
+ ctrl->scd_local->in_use = 0;
+ if (ctrl->scd_local->invalid)
+ {
+ err = npth_mutex_lock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to acquire the start_scd lock: %s\n",
+ strerror (err));
+ return gpg_error (GPG_ERR_INTERNAL);
+ }
+
+ assuan_release (ctrl->scd_local->ctx);
+ ctrl->scd_local->ctx = NULL;
+
+ err = npth_mutex_unlock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to release the start_scd lock: %s\n",
+ strerror (err));
+ return gpg_error (GPG_ERR_INTERNAL);
+ }
+ }
+ ctrl->scd_local->invalid = 0;
return rc;
}
@@ -216,15 +238,12 @@ start_scd (ctrl_t ctrl)
scd_local_list = ctrl->scd_local;
}
-
- /* Assert that the lock count is as expected. */
- if (ctrl->scd_local->locked)
+ if (ctrl->scd_local->in_use)
{
- log_error ("start_scd: invalid lock count (%d)\n",
- ctrl->scd_local->locked);
+ log_error ("start_scd: CTX is in use\n");
return gpg_error (GPG_ERR_INTERNAL);
}
- ctrl->scd_local->locked++;
+ ctrl->scd_local->in_use = 1;
if (ctrl->scd_local->ctx)
return 0; /* Okay, the context is fine. We used to test for an
@@ -344,7 +363,7 @@ start_scd (ctrl_t ctrl)
detached flag so that under Windows SCDAEMON does not show up a
new window. */
rc = assuan_pipe_connect (ctx, opt.scdaemon_program, argv,
- no_close_list, atfork_cb, NULL,
+ no_close_list, atfork_cb, NULL,
ASSUAN_PIPE_CONNECT_DETACHED);
if (rc)
{
@@ -414,7 +433,7 @@ start_scd (ctrl_t ctrl)
{
unlock_scd (ctrl, err);
if (ctx)
- assuan_release (ctx);
+ assuan_release (ctx);
}
else
{
@@ -495,14 +514,13 @@ agent_scd_check_aliveness (void)
struct scd_local_s *sl;
assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
- assuan_release (primary_scd_ctx);
for (sl=scd_local_list; sl; sl = sl->next_local)
{
- if (sl->ctx)
+ sl->invalid = 1;
+ if (!sl->in_use && sl->ctx)
{
- if (sl->ctx != primary_scd_ctx)
- assuan_release (sl->ctx);
+ assuan_release (sl->ctx);
sl->ctx = NULL;
}
}
commit f45d6124696cc92ccdec09841b8182679c377997
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 10:37:02 2018 +0900
agent: Clean up SCDaemon management.
* agent/call-scd.c (struct scd_local_s): Remove ctrl_backlink.
(start_scd): Don't assign to the field.
(agent_scd_check_aliveness): Fix typo in comment.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 51d9abd..954be02 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -54,12 +54,6 @@ struct scd_local_s
SCD_LOCAL_LIST (see below). */
struct scd_local_s *next_local;
- /* We need to get back to the ctrl object actually referencing this
- structure. This is really an awkward way of enumerating the local
- contexts. A much cleaner way would be to keep a global list of
- ctrl objects to enumerate them. */
- ctrl_t ctrl_backlink;
-
assuan_context_t ctx; /* NULL or session context for the SCdaemon
used with this connection. */
int locked; /* This flag is used to assert proper use of
@@ -218,7 +212,6 @@ start_scd (ctrl_t ctrl)
ctrl->scd_local = xtrycalloc (1, sizeof *ctrl->scd_local);
if (!ctrl->scd_local)
return gpg_error_from_syserror ();
- ctrl->scd_local->ctrl_backlink = ctrl;
ctrl->scd_local->next_local = scd_local_list;
scd_local_list = ctrl->scd_local;
}
@@ -497,7 +490,7 @@ agent_scd_check_aliveness (void)
now but take care that it won't do another wait. Also
cleanup all other connections and release their
resources. The next use will start a new daemon then.
- Due to the use of the START_SCD_LOCAL we are sure that
+ Due to the use of the START_SCD_LOCK we are sure that
none of these context are actually in use. */
struct scd_local_s *sl;
-----------------------------------------------------------------------
Summary of changes:
agent/agent.h | 1 -
agent/call-scd.c | 245 +++++++++++++++++++++++++++++-------------------------
agent/gpg-agent.c | 3 -
3 files changed, 134 insertions(+), 115 deletions(-)
hooks/post-receive
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".
The branch, master has been updated
via 40c7923ea881881a48de8f303b0870ec5910e13d (commit)
via 9fb3f0f3f79e74166cce8e0781e97043f25890cc (commit)
via f45d6124696cc92ccdec09841b8182679c377997 (commit)
from fa1b1eaa4241ff3f0634c8bdf8591cbc7c464144 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 40c7923ea881881a48de8f303b0870ec5910e13d
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 12:07:36 2018 +0900
agent: Have a thread to wait for the child process of scdaemon.
* agent/call-scd.c (wait_child_thread): New.
(start_scd): Create a thread for wait_child_thread.
(agent_scd_check_aliveness): Remove.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/agent.h b/agent/agent.h
index 9baf596..8a3e5c6 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -546,7 +546,6 @@ int divert_writekey (ctrl_t ctrl, int force, const char *serialno,
void initialize_module_call_scd (void);
void agent_scd_dump_state (void);
int agent_scd_check_running (void);
-void agent_scd_check_aliveness (void);
int agent_reset_scd (ctrl_t ctrl);
int agent_card_learn (ctrl_t ctrl,
void (*kpinfo_cb)(void*, const char *),
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 592ddb9..c6d062a 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -207,6 +207,85 @@ atfork_cb (void *opaque, int where)
}
+static void *
+wait_child_thread (void *arg)
+{
+ int err;
+ struct scd_local_s *sl;
+
+#ifdef HAVE_W32_SYSTEM
+ HANDLE pid = (HANDLE)arg;
+
+ npth_unprotect ();
+ WaitForSingleObject ((HANDLE)pid, INFINITE);
+ npth_protect ();
+#else
+ int wstatus;
+ pid_t pid = (pid_t)(uintptr_t)arg;
+
+ again:
+ npth_unprotect ();
+ err = waitpid (pid, &wstatus, 0);
+ npth_protect ();
+
+ if (err < 0)
+ {
+ if (errno == EINTR)
+ goto again;
+ log_error ("waitpid failed: %s\n", strerror (errno));
+ return NULL;
+ }
+ else
+ {
+ if (WIFEXITED (wstatus))
+ log_info ("scdaemon finished (status %d)\n", WEXITSTATUS (wstatus));
+ else if (WIFSIGNALED (wstatus))
+ log_info ("scdaemon killed by signal %d\n", WTERMSIG (wstatus));
+ else
+ {
+ if (WIFSTOPPED (wstatus))
+ log_info ("scdaemon stopped by signal %d\n", WSTOPSIG (wstatus));
+ goto again;
+ }
+ }
+#endif
+
+ err = npth_mutex_lock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to acquire the start_scd lock: %s\n",
+ strerror (err));
+ }
+ else
+ {
+ assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
+
+ for (sl = scd_local_list; sl; sl = sl->next_local)
+ {
+ sl->invalid = 1;
+ if (!sl->in_use && sl->ctx)
+ {
+ assuan_release (sl->ctx);
+ sl->ctx = NULL;
+ }
+ }
+
+ primary_scd_ctx = NULL;
+ primary_scd_ctx_reusable = 0;
+
+ xfree (socket_name);
+ socket_name = NULL;
+
+ err = npth_mutex_unlock (&start_scd_lock);
+ if (err)
+ log_error ("failed to release the start_scd lock while"
+ " doing the aliveness check: %s\n", strerror (err));
+ }
+
+ return NULL;
+}
+
+
/* Fork off the SCdaemon if this has not already been done. Lock the
daemon and make sure that a proper context has been setup in CTRL.
This function might also lock the daemon, which means that the
@@ -427,6 +506,24 @@ start_scd (ctrl_t ctrl)
primary_scd_ctx = ctx;
primary_scd_ctx_reusable = 0;
+ {
+ npth_t thread;
+ npth_attr_t tattr;
+ pid_t pid;
+
+ pid = assuan_get_pid (primary_scd_ctx);
+ err = npth_attr_init (&tattr);
+ if (!err)
+ {
+ npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED);
+ err = npth_create (&thread, &tattr, wait_child_thread,
+ (void *)(uintptr_t)pid);
+ if (err)
+ log_error ("error spawning wait_child_thread: %s\n", strerror (err));
+ npth_attr_destroy (&tattr);
+ }
+ }
+
leave:
xfree (abs_homedir);
if (err)
@@ -456,91 +553,6 @@ agent_scd_check_running (void)
}
-/* Check whether the Scdaemon is still alive and clean it up if not. */
-void
-agent_scd_check_aliveness (void)
-{
- pid_t pid;
-#ifdef HAVE_W32_SYSTEM
- DWORD rc;
-#else
- int rc;
-#endif
- struct timespec abstime;
- int err;
-
- if (!primary_scd_ctx)
- return; /* No scdaemon running. */
-
- /* This is not a critical function so we use a short timeout while
- acquiring the lock. */
- npth_clock_gettime (&abstime);
- abstime.tv_sec += 1;
- err = npth_mutex_timedlock (&start_scd_lock, &abstime);
- if (err)
- {
- if (err == ETIMEDOUT)
- {
- if (opt.verbose > 1)
- log_info ("failed to acquire the start_scd lock while"
- " doing an aliveness check: %s\n", strerror (err));
- }
- else
- log_error ("failed to acquire the start_scd lock while"
- " doing an aliveness check: %s\n", strerror (err));
- return;
- }
-
- if (primary_scd_ctx)
- {
- pid = assuan_get_pid (primary_scd_ctx);
-#ifdef HAVE_W32_SYSTEM
- /* If we have a PID we disconnect if either GetExitProcessCode
- fails or if ir returns the exit code of the scdaemon. 259 is
- the error code for STILL_ALIVE. */
- if (pid != (pid_t)(void*)(-1) && pid
- && (!GetExitCodeProcess ((HANDLE)pid, &rc) || rc != 259))
-#else
- if (pid != (pid_t)(-1) && pid
- && ((rc=waitpid (pid, NULL, WNOHANG))==-1 || (rc == pid)) )
-#endif
- {
- /* Okay, scdaemon died. Disconnect the primary connection
- now but take care that it won't do another wait. Also
- cleanup all other connections and release their
- resources. The next use will start a new daemon then.
- Due to the use of the START_SCD_LOCK we are sure that
- none of these context are actually in use. */
- struct scd_local_s *sl;
-
- assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
-
- for (sl=scd_local_list; sl; sl = sl->next_local)
- {
- sl->invalid = 1;
- if (!sl->in_use && sl->ctx)
- {
- assuan_release (sl->ctx);
- sl->ctx = NULL;
- }
- }
-
- primary_scd_ctx = NULL;
- primary_scd_ctx_reusable = 0;
-
- xfree (socket_name);
- socket_name = NULL;
- }
- }
-
- err = npth_mutex_unlock (&start_scd_lock);
- if (err)
- log_error ("failed to release the start_scd lock while"
- " doing the aliveness check: %s\n", strerror (err));
-}
-
-
-
/* Reset the SCD if it has been used. Actually it is not a reset but
a cleanup of resources used by the current connection. */
int
diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c
index 911064c..de6947d 100644
--- a/agent/gpg-agent.c
+++ b/agent/gpg-agent.c
@@ -2370,9 +2370,6 @@ handle_tick (void)
if (!last_minute)
last_minute = time (NULL);
- /* Check whether the scdaemon has died and cleanup in this case. */
- agent_scd_check_aliveness ();
-
/* If we are running as a child of another process, check whether
the parent is still alive and shutdown if not. */
#ifndef HAVE_W32_SYSTEM
commit 9fb3f0f3f79e74166cce8e0781e97043f25890cc
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 11:05:28 2018 +0900
agent: Defer calling assuan_release when it's still in use.
* agent/call-scd.c (struct scd_local_s): Remove LOCK, introduce IN_USE
and INVALID flags.
(unlock_scd): Call assuan_release when CTX is invalid.
(start_scd): Set IN_USE.
(agent_scd_check_aliveness): Don't call assuan_release when it's in use.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 954be02..592ddb9 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -54,11 +54,10 @@ struct scd_local_s
SCD_LOCAL_LIST (see below). */
struct scd_local_s *next_local;
- assuan_context_t ctx; /* NULL or session context for the SCdaemon
- used with this connection. */
- int locked; /* This flag is used to assert proper use of
- start_scd and unlock_scd. */
-
+ assuan_context_t ctx; /* NULL or session context for the SCdaemon
+ used with this connection. */
+ unsigned int in_use: 1; /* CTX is in use. */
+ unsigned int invalid:1; /* CTX is invalid, should be released. */
};
@@ -132,7 +131,7 @@ initialize_module_call_scd (void)
{
err = npth_mutex_init (&start_scd_lock, NULL);
if (err)
- log_fatal ("error initializing mutex: %s\n", strerror (err));
+ log_fatal ("error initializing mutex: %s\n", strerror (err));
initialized = 1;
}
}
@@ -162,14 +161,37 @@ agent_scd_dump_state (void)
static int
unlock_scd (ctrl_t ctrl, int rc)
{
- if (ctrl->scd_local->locked != 1)
+ int err;
+
+ if (ctrl->scd_local->in_use == 0)
{
- log_error ("unlock_scd: invalid lock count (%d)\n",
- ctrl->scd_local->locked);
+ log_error ("unlock_scd: CTX is not in use\n");
if (!rc)
rc = gpg_error (GPG_ERR_INTERNAL);
}
- ctrl->scd_local->locked = 0;
+ ctrl->scd_local->in_use = 0;
+ if (ctrl->scd_local->invalid)
+ {
+ err = npth_mutex_lock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to acquire the start_scd lock: %s\n",
+ strerror (err));
+ return gpg_error (GPG_ERR_INTERNAL);
+ }
+
+ assuan_release (ctrl->scd_local->ctx);
+ ctrl->scd_local->ctx = NULL;
+
+ err = npth_mutex_unlock (&start_scd_lock);
+ if (err)
+ {
+ log_error ("failed to release the start_scd lock: %s\n",
+ strerror (err));
+ return gpg_error (GPG_ERR_INTERNAL);
+ }
+ }
+ ctrl->scd_local->invalid = 0;
return rc;
}
@@ -216,15 +238,12 @@ start_scd (ctrl_t ctrl)
scd_local_list = ctrl->scd_local;
}
-
- /* Assert that the lock count is as expected. */
- if (ctrl->scd_local->locked)
+ if (ctrl->scd_local->in_use)
{
- log_error ("start_scd: invalid lock count (%d)\n",
- ctrl->scd_local->locked);
+ log_error ("start_scd: CTX is in use\n");
return gpg_error (GPG_ERR_INTERNAL);
}
- ctrl->scd_local->locked++;
+ ctrl->scd_local->in_use = 1;
if (ctrl->scd_local->ctx)
return 0; /* Okay, the context is fine. We used to test for an
@@ -344,7 +363,7 @@ start_scd (ctrl_t ctrl)
detached flag so that under Windows SCDAEMON does not show up a
new window. */
rc = assuan_pipe_connect (ctx, opt.scdaemon_program, argv,
- no_close_list, atfork_cb, NULL,
+ no_close_list, atfork_cb, NULL,
ASSUAN_PIPE_CONNECT_DETACHED);
if (rc)
{
@@ -414,7 +433,7 @@ start_scd (ctrl_t ctrl)
{
unlock_scd (ctrl, err);
if (ctx)
- assuan_release (ctx);
+ assuan_release (ctx);
}
else
{
@@ -495,14 +514,13 @@ agent_scd_check_aliveness (void)
struct scd_local_s *sl;
assuan_set_flag (primary_scd_ctx, ASSUAN_NO_WAITPID, 1);
- assuan_release (primary_scd_ctx);
for (sl=scd_local_list; sl; sl = sl->next_local)
{
- if (sl->ctx)
+ sl->invalid = 1;
+ if (!sl->in_use && sl->ctx)
{
- if (sl->ctx != primary_scd_ctx)
- assuan_release (sl->ctx);
+ assuan_release (sl->ctx);
sl->ctx = NULL;
}
}
commit f45d6124696cc92ccdec09841b8182679c377997
Author: NIIBE Yutaka <***@fsij.org>
Date: Mon Nov 26 10:37:02 2018 +0900
agent: Clean up SCDaemon management.
* agent/call-scd.c (struct scd_local_s): Remove ctrl_backlink.
(start_scd): Don't assign to the field.
(agent_scd_check_aliveness): Fix typo in comment.
Signed-off-by: NIIBE Yutaka <***@fsij.org>
diff --git a/agent/call-scd.c b/agent/call-scd.c
index 51d9abd..954be02 100644
--- a/agent/call-scd.c
+++ b/agent/call-scd.c
@@ -54,12 +54,6 @@ struct scd_local_s
SCD_LOCAL_LIST (see below). */
struct scd_local_s *next_local;
- /* We need to get back to the ctrl object actually referencing this
- structure. This is really an awkward way of enumerating the local
- contexts. A much cleaner way would be to keep a global list of
- ctrl objects to enumerate them. */
- ctrl_t ctrl_backlink;
-
assuan_context_t ctx; /* NULL or session context for the SCdaemon
used with this connection. */
int locked; /* This flag is used to assert proper use of
@@ -218,7 +212,6 @@ start_scd (ctrl_t ctrl)
ctrl->scd_local = xtrycalloc (1, sizeof *ctrl->scd_local);
if (!ctrl->scd_local)
return gpg_error_from_syserror ();
- ctrl->scd_local->ctrl_backlink = ctrl;
ctrl->scd_local->next_local = scd_local_list;
scd_local_list = ctrl->scd_local;
}
@@ -497,7 +490,7 @@ agent_scd_check_aliveness (void)
now but take care that it won't do another wait. Also
cleanup all other connections and release their
resources. The next use will start a new daemon then.
- Due to the use of the START_SCD_LOCAL we are sure that
+ Due to the use of the START_SCD_LOCK we are sure that
none of these context are actually in use. */
struct scd_local_s *sl;
-----------------------------------------------------------------------
Summary of changes:
agent/agent.h | 1 -
agent/call-scd.c | 245 +++++++++++++++++++++++++++++-------------------------
agent/gpg-agent.c | 3 -
3 files changed, 134 insertions(+), 115 deletions(-)
hooks/post-receive
--
The GNU Privacy Guard
http://git.gnupg.org
The GNU Privacy Guard
http://git.gnupg.org