jack andrews wrote:
> #At file:///C:/w/repo/70/ based on revid:jack@asus-20090317085019-yes00fe6c39b0fuu
>
> 2861 jack andrews 2009-03-19
> bug#37225 ndbd and ndb_mgmd pid file not removed after shutdown
>
> modified:
> storage/ndb/src/common/portlib/my_daemon.cc
> === modified file 'storage/ndb/src/common/portlib/my_daemon.cc'
> --- a/storage/ndb/src/common/portlib/my_daemon.cc 2009-03-13 09:48:24 +0000
> +++ b/storage/ndb/src/common/portlib/my_daemon.cc 2009-03-19 05:40:06 +0000
> @@ -51,7 +51,8 @@ static char** g_argv;
> static HANDLE g_shutdown_event;
> static my_daemon_stop_t g_stop_func;
> static my_daemon_run_t g_run_func;
> -
> +static const char *g_pidfile_name = 0;
> +static int g_pidfd = -1, g_logfd = -1;
magnus: Need to be put outside "#ifdef WIN"
>
> static void stopper_thread(void*)
> {
> @@ -293,6 +294,9 @@ check_files(const char *pidfile_name,
> static int
> do_files(const char *pidfile_name, int pidfd, int logfd)
> {
> + g_pidfd = pidfd;
> + g_logfd = logfd;
> +
magnus: I would prefer if the fd's are saved immediately after they have
been successfully opened. Which means they should probably be set in
two different places.
> /* Lock the lock file */
> if (lockf(pidfd, F_LOCK, 0) == -1)
> return ERR1("Failed to lock pidfile '%s', errno: %d",
> @@ -334,6 +338,9 @@ do_files(const char *pidfile_name, int p
> int my_daemonize(const char* pidfile_name, const char *logfile_name)
> {
> int pidfd = -1, logfd = -1;
> +
> + g_pidfile_name = pidfile_name;
> +
magnus: same with this, set it after the pidfile has been successfully
created.
> if (check_files(pidfile_name, logfile_name, &pidfd, &logfd))
> return 1;
>
> @@ -358,9 +365,32 @@ int my_daemonize(const char* pidfile_nam
> return 0;
> }
>
> +int my_daemon_close(int fd)
magnus:
1. should be a static function and as such it does not necessarily need
to be prefixed with my_daemon, maybe 'safe_close'?
2. The function can be made void as well, since we don't care about
return code at all.
3. Maybe we can just use 'close' directly? I mean, how likely it is to
get EINTR? If that is not handled, nothing serious happen since we are
going to terminate the process anyway.
> +{
> + int err;
> +
> + do
> + {
> + err= close(fd);
> + } while (err == -1 && errno == EINTR);
> +
> + if (err)
> + return ERR1("Failed to close, errno: %d", errno);
> +
> + return 0;
> +}
>
> void my_daemon_exit(int status)
> {
> + if (g_pidfd != -1)
> + my_daemon_close(g_pidfd);
> +
> + if (g_logfd != -1)
> + my_daemon_close(g_logfd);
> +
> + if (g_pidfile_name)
> + my_delete(g_pidfile_name, MYF(0));
magnus: I think you should just to do a call to 'unlink', same
motivation as above. If unlink fails, there is not much we can do.
And write a "static inline" function for windows that map unlink to
_unlink(see 'truncate' and 'lockf').
> +
> #ifdef _WIN32
> /*
> Stop by calling NtService::Stop if running
> @@ -369,5 +399,6 @@ void my_daemon_exit(int status)
> if (g_shutdown_event)
> g_ntsvc.Stop();
> #endif
> +
> exit(status);
> }
>
>
>
> ------------------------------------------------------------------------
>
>
