From 8991f6ea59eb9870dfeb006a4cf94b31c075cfa9 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Wed, 21 Jun 2017 17:54:04 +0500 Subject: [PATCH] Fixing: [pgpool-hackers: 2390] Problems with the relative paths in daemon mode Pgpool-II does all the path related calculations based on CWD (current working directory) which only works as expected and intended until the CWD does not change. But in daemon mode the first thing Pgpool-II do after becoming the daemon process is, it changes it's CWD to system root ("/") and after that all the relative paths becomes invalid. This means in daemon mode if the pgpool.conf file is specified as an argument using the relative path, Pgpool-II will not be able to find the pool_passwd or other required file whose location depends on the location of pgpool.conf file, and will eventually fail to start, or even worst may read/write some wrong file. The solution to this is to convert the relative path of the pgpool.conf and all file paths provided in the argument list to the Pgpool-II to the absolute paths at the startup and later use those absolute paths for all path related calculations. Apart from using the absolute paths for pgpool.conf, pcp.conf and pool_hba.conf files, The commit also modifies the behaviour of pid_file_name configuration parameter, when the relative path is used for this config in pgpool.conf, that relative path is considered with relative to the pgpool.conf path, instead of the current working directory. --- src/include/utils/pool_path.h | 4 +- src/main/main.c | 107 ++++++++++++++++++--- src/main/pgpool_main.c | 6 +- src/redhat/pgpool.conf.sample.patch | 6 ++ src/sample/pgpool.conf.sample | 3 + src/sample/pgpool.conf.sample-master-slave | 3 + src/sample/pgpool.conf.sample-replication | 3 + src/sample/pgpool.conf.sample-stream | 3 + src/utils/pool_path.c | 88 ++++++++++++++++- 9 files changed, 205 insertions(+), 18 deletions(-) diff --git a/src/include/utils/pool_path.h b/src/include/utils/pool_path.h index 1348cc695..1b3b53d6d 100644 --- a/src/include/utils/pool_path.h +++ b/src/include/utils/pool_path.h @@ -48,5 +48,7 @@ extern void get_parent_directory(char *path); extern void join_path_components(char *ret_path, const char *head, const char *tail); extern void canonicalize_path(char *path); - +extern char *get_current_working_dir(void); +extern char *make_absolute_path(const char *path, const char* base_dir); + #endif /* POOL_PATH_H */ diff --git a/src/main/main.c b/src/main/main.c index 852d28882..91544c248 100644 --- a/src/main/main.c +++ b/src/main/main.c @@ -38,6 +38,7 @@ #include "utils/elog.h" #include "utils/palloc.h" #include "utils/memutils.h" +#include "utils/pool_path.h" #include "version.h" #include "auth/pool_passwd.h" @@ -45,6 +46,7 @@ #include "watchdog/wd_ext.h" static void daemonize(void); +static char *get_pid_file_path(void); static int read_pid_file(void); static void write_pid_file(void); static void usage(void); @@ -52,9 +54,10 @@ static void show_version(void); static void stop_me(void); static void FileUnlink(int code, Datum path); -char pcp_conf_file[POOLMAXPATHLEN+1]; /* path for pcp.conf */ -char conf_file[POOLMAXPATHLEN+1]; -char hba_file[POOLMAXPATHLEN+1]; +char *pcp_conf_file = NULL; /* absolute path of the pcp.conf */ +char *conf_file = NULL; /* absolute path of the pgpool.conf */ +char *hba_file = NULL; /* absolute path of the hba.conf */ +char *base_dir = NULL; /* The working dir from where pgpool was invoked from */ static int not_detach = 0; /* non 0 if non detach option (-n) is given */ int stop_sig = SIGTERM; /* stopping signal default value */ @@ -69,6 +72,10 @@ int main(int argc, char **argv) bool discard_status = false; bool clear_memcache_oidmaps = false; + char pcp_conf_file_path[POOLMAXPATHLEN+1]; + char conf_file_path[POOLMAXPATHLEN+1]; + char hba_file_path[POOLMAXPATHLEN+1]; + static struct option long_options[] = { {"hba-file", required_argument, NULL, 'a'}, {"debug", no_argument, NULL, 'd'}, @@ -87,9 +94,9 @@ int main(int argc, char **argv) myargc = argc; myargv = argv; - snprintf(conf_file, sizeof(conf_file), "%s/%s", DEFAULT_CONFIGDIR, POOL_CONF_FILE_NAME); - snprintf(pcp_conf_file, sizeof(pcp_conf_file), "%s/%s", DEFAULT_CONFIGDIR, PCP_PASSWD_FILE_NAME); - snprintf(hba_file, sizeof(hba_file), "%s/%s", DEFAULT_CONFIGDIR, HBA_CONF_FILE_NAME); + snprintf(conf_file_path, sizeof(conf_file_path), "%s/%s", DEFAULT_CONFIGDIR, POOL_CONF_FILE_NAME); + snprintf(pcp_conf_file_path, sizeof(pcp_conf_file_path), "%s/%s", DEFAULT_CONFIGDIR, PCP_PASSWD_FILE_NAME); + snprintf(hba_file_path, sizeof(hba_file_path), "%s/%s", DEFAULT_CONFIGDIR, HBA_CONF_FILE_NAME); while ((opt = getopt_long(argc, argv, "a:df:F:hm:nDCxv", long_options, &optindex)) != -1) { switch (opt) @@ -100,7 +107,7 @@ int main(int argc, char **argv) usage(); exit(1); } - strlcpy(hba_file, optarg, sizeof(hba_file)); + strlcpy(hba_file_path, optarg, sizeof(hba_file_path)); break; case 'x': /* enable cassert */ @@ -117,7 +124,7 @@ int main(int argc, char **argv) usage(); exit(1); } - strlcpy(conf_file, optarg, sizeof(conf_file)); + strlcpy(conf_file_path, optarg, sizeof(conf_file_path)); break; case 'F': /* specify PCP password file */ @@ -126,7 +133,7 @@ int main(int argc, char **argv) usage(); exit(1); } - strlcpy(pcp_conf_file, optarg, sizeof(pcp_conf_file)); + strlcpy(pcp_conf_file_path, optarg, sizeof(pcp_conf_file_path)); break; case 'h': @@ -184,6 +191,13 @@ int main(int argc, char **argv) /* create MemoryContexts */ MemoryContextInit(); + /* load the CWD before it is changed */ + base_dir = get_current_working_dir(); + /* convert all the paths to absolute paths*/ + conf_file = make_absolute_path(conf_file_path, base_dir); + pcp_conf_file = make_absolute_path(pcp_conf_file_path, base_dir); + hba_file = make_absolute_path(hba_file_path, base_dir); + mypid = getpid(); pool_init_config(); @@ -231,7 +245,6 @@ int main(int argc, char **argv) if (!strcmp(argv[optind], "stop")) { stop_me(); - unlink(pool_config->pid_file_name); exit(0); } else @@ -422,6 +435,7 @@ static void daemonize(void) static void stop_me(void) { pid_t pid; + char *pid_file; pid = read_pid_file(); if (pid < 0) @@ -445,6 +459,49 @@ static void stop_me(void) sleep(1); } fprintf(stderr, "done.\n"); + pid_file = get_pid_file_path(); + unlink(pid_file); + pfree(pid_file); +} + +/* + * The function returns the palloc'd copy of pid_file_path, + * caller must free it after use + */ +static char *get_pid_file_path(void) +{ + char *new = NULL; + if (!is_absolute_path(pool_config->pid_file_name)) + { + /* + * some implementations of dirname() may modify the + * string argument passed to it, so do not use the original + * conf_file as an argument + */ + char *conf_file_copy = pstrdup(conf_file); + char *conf_dir = dirname(conf_file_copy); + size_t path_size; + if (conf_dir == NULL) + { + ereport(LOG, + (errmsg("failed to get the dirname of pid file:\"%s\". reason: %s", + pool_config->pid_file_name, strerror(errno)))); + return NULL; + } + path_size = strlen(conf_dir) + strlen(pool_config->pid_file_name) + 1 + 1; + new = palloc(path_size); + snprintf(new, path_size, "%s/%s",conf_dir, pool_config->pid_file_name); + + ereport(DEBUG1, + (errmsg("pid file location is \"%s\"", + new))); + } + else + { + new = pstrdup(pool_config->pid_file_name); + } + + return new; } /* @@ -455,15 +512,25 @@ static int read_pid_file(void) int fd; int readlen; char pidbuf[128]; + char *pid_file = get_pid_file_path(); - fd = open(pool_config->pid_file_name, O_RDONLY); + if (pid_file == NULL) + { + ereport(FATAL, + (errmsg("failed to read pid file"), + errdetail("failed to get pid file path from \"%s\"", + pool_config->pid_file_name))); + } + fd = open(pid_file, O_RDONLY); if (fd == -1) { + pfree(pid_file); return -1; } if ((readlen = read(fd, pidbuf, sizeof(pidbuf))) == -1) { close(fd); + pfree(pid_file); ereport(FATAL, (errmsg("could not read pid file as \"%s\". reason: %s", pool_config->pid_file_name, strerror(errno)))); @@ -471,10 +538,12 @@ static int read_pid_file(void) else if (readlen == 0) { close(fd); + pfree(pid_file); ereport(FATAL, (errmsg("EOF detected while reading pid file \"%s\". reason: %s", pool_config->pid_file_name, strerror(errno)))); } + pfree(pid_file); close(fd); return(atoi(pidbuf)); } @@ -486,8 +555,17 @@ static void write_pid_file(void) { int fd; char pidbuf[128]; + char *pid_file = get_pid_file_path(); + + if (pid_file == NULL) + { + ereport(FATAL, + (errmsg("failed to write pid file"), + errdetail("failed to get pid file path from \"%s\"", + pool_config->pid_file_name))); + } - fd = open(pool_config->pid_file_name, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR); + fd = open(pid_file, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR); if (fd == -1) { ereport(FATAL, @@ -498,6 +576,7 @@ static void write_pid_file(void) if (write(fd, pidbuf, strlen(pidbuf)+1) == -1) { close(fd); + pfree(pid_file); ereport(FATAL, (errmsg("could not write pid file as %s. reason: %s", pool_config->pid_file_name, strerror(errno)))); @@ -505,18 +584,20 @@ static void write_pid_file(void) if (fsync(fd) == -1) { close(fd); + pfree(pid_file); ereport(FATAL, (errmsg("could not fsync pid file as %s. reason: %s", pool_config->pid_file_name, strerror(errno)))); } if (close(fd) == -1) { + pfree(pid_file); ereport(FATAL, (errmsg("could not close pid file as %s. reason: %s", pool_config->pid_file_name, strerror(errno)))); } /* register the call back to delete the pid file at system exit */ - on_proc_exit(FileUnlink, (Datum) pool_config->pid_file_name); + on_proc_exit(FileUnlink, (Datum) pid_file); } /* diff --git a/src/main/pgpool_main.c b/src/main/pgpool_main.c index bf10f9ce0..cf731b072 100644 --- a/src/main/pgpool_main.c +++ b/src/main/pgpool_main.c @@ -151,9 +151,9 @@ static int *fds; /* listening file descriptors (UNIX socket, inet domain sockets static int pcp_unix_fd; /* unix domain socket fd for PCP (not used) */ static int pcp_inet_fd; /* inet domain socket fd for PCP */ -extern char pcp_conf_file[POOLMAXPATHLEN+1]; /* path for pcp.conf */ -extern char conf_file[POOLMAXPATHLEN+1]; -extern char hba_file[POOLMAXPATHLEN+1]; +extern char *pcp_conf_file; /* path for pcp.conf */ +extern char *conf_file; +extern char *hba_file; static int exiting = 0; /* non 0 if I'm exiting */ static int switching = 0; /* non 0 if I'm fail overing or degenerating */ diff --git a/src/redhat/pgpool.conf.sample.patch b/src/redhat/pgpool.conf.sample.patch index d47674b61..515f26fc3 100644 --- a/src/redhat/pgpool.conf.sample.patch +++ b/src/redhat/pgpool.conf.sample.patch @@ -37,6 +37,9 @@ *** 214,220 **** pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # as an absolute path # (change requires restart) ! logdir = '/tmp' # Directory of pgPool status file @@ -45,6 +48,9 @@ --- 214,220 ---- pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # or as absolute path # (change requires restart) ! logdir = '/var/log/pgpool' # Directory of pgPool status file diff --git a/src/sample/pgpool.conf.sample b/src/sample/pgpool.conf.sample index 54fa4c63f..8bab29926 100644 --- a/src/sample/pgpool.conf.sample +++ b/src/sample/pgpool.conf.sample @@ -213,6 +213,9 @@ debug_level = 0 pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # as an absolute path # (change requires restart) logdir = '/var/log/pgpool' # Directory of pgPool status file diff --git a/src/sample/pgpool.conf.sample-master-slave b/src/sample/pgpool.conf.sample-master-slave index 4a13f1122..6975fa10a 100644 --- a/src/sample/pgpool.conf.sample-master-slave +++ b/src/sample/pgpool.conf.sample-master-slave @@ -213,6 +213,9 @@ debug_level = 0 pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # as an absolute path # (change requires restart) logdir = '/tmp' # Directory of pgPool status file diff --git a/src/sample/pgpool.conf.sample-replication b/src/sample/pgpool.conf.sample-replication index ccbd0e295..b0d50d04d 100644 --- a/src/sample/pgpool.conf.sample-replication +++ b/src/sample/pgpool.conf.sample-replication @@ -213,6 +213,9 @@ debug_level = 0 pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # as an absolute path # (change requires restart) logdir = '/tmp' # Directory of pgPool status file diff --git a/src/sample/pgpool.conf.sample-stream b/src/sample/pgpool.conf.sample-stream index 6ff700e81..13804cf3a 100644 --- a/src/sample/pgpool.conf.sample-stream +++ b/src/sample/pgpool.conf.sample-stream @@ -214,6 +214,9 @@ debug_level = 0 pid_file_name = '/var/run/pgpool/pgpool.pid' # PID file name + # Can be specified as relative to the" + # location of pgpool.conf file or + # as an absolute path # (change requires restart) logdir = '/tmp' # Directory of pgPool status file diff --git a/src/utils/pool_path.c b/src/utils/pool_path.c index 46ab446e6..63bc2e936 100644 --- a/src/utils/pool_path.c +++ b/src/utils/pool_path.c @@ -6,7 +6,7 @@ * pgpool: a language independent connection pool server for PostgreSQL * written by Tatsuo Ishii * - * Portions Copyright (c) 2003-2008, PgPool Global Development Group + * Portions Copyright (c) 2003-2017, PgPool Global Development Group * Portions Copyright (c) 2004, PostgreSQL Global Development Group * * Permission to use, copy, modify, and distribute this software and @@ -28,6 +28,12 @@ #include "utils/pool_path.h" #include #include +#include +#include +#include +#include "utils/elog.h" +#include "utils/palloc.h" +#include "utils/memutils.h" static void trim_directory(char *path); static void trim_trailing_separator(char *path); @@ -191,3 +197,83 @@ static void trim_trailing_separator(char *path) for (p--; p > path && IS_DIR_SEP(*p); p--) *p = '\0'; } + +char * +get_current_working_dir(void) +{ + char *buf = NULL; + size_t buflen = MAXPGPATH; + + for (;;) + { + buf = palloc(buflen); + + if (getcwd(buf, buflen)) + break; + + else if (errno == ERANGE) + { + pfree(buf); + buflen *= 2; + continue; + } + else + { + int save_errno = errno; + + pfree(buf); + errno = save_errno; + ereport(ERROR, + (errmsg("could not get current working directory: %m"))); + return NULL; + } + } + return buf; +} +/* + * make_absolute_path + * + * If the given pathname isn't already absolute, make it so, interpreting + * it relative to the current working directory. + * + * if arg base_dir is NULL, The function gets the current cwd + * Also canonicalizes the path. The result is always a palloc'd copy. + * + * Logic borrowed from PostgreSQL source + */ +char * +make_absolute_path(const char *path, const char* base_dir) +{ + char *new; + /* Returning null for null input is convenient for some callers */ + if (path == NULL) + return NULL; + + if (!is_absolute_path(path)) + { + const char *cwd = NULL; + if (base_dir == NULL) + { + cwd = get_current_working_dir(); + if (!cwd) + return NULL; + } + else + cwd = base_dir; + + new = palloc(strlen(cwd) + strlen(path) + 2); + sprintf(new, "%s/%s", cwd, path); + + if (!base_dir) + pfree((void*)cwd); + } + else + { + new = pstrdup(path); + } + + /* Make sure punctuation is canonical, too */ + canonicalize_path(new); + + return new; +} -- 2.39.5