Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FvwmMFL: fix singletion/locking mechanism #1057

Merged
merged 1 commit into from
Aug 22, 2024
Merged

FvwmMFL: fix singletion/locking mechanism #1057

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 35 additions & 93 deletions modules/FvwmMFL/FvwmMFL.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "libs/envvar.h"
#include "libs/cJSON.h"

#include <sys/file.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/time.h>
Expand Down Expand Up @@ -52,7 +53,8 @@
static int debug;
struct fvwm_msg;
static char *socket_name, *socket_basepath;
static char *pid_file;
static char *lockfile = NULL;
static int fvwmmfl_lock_fd = -1;

struct client {
struct bufferevent *comms;
Expand Down Expand Up @@ -122,11 +124,8 @@ static struct fvwm_msg *fvwm_msg_new(void);
static void fvwm_msg_free(struct fvwm_msg *);
static void register_interest(void);
static void send_version_info(struct client *);
static void set_socket_pathname(void);
static int check_pid(void);
static void set_pid_file(void);
static void create_pid_file(void);
static void delete_pid_file(void);
static void set_pathnames(void);
static int lock_fvwmmfl(void);
static void fm_to_json(struct fvwm_msg *);

static void
Expand All @@ -138,88 +137,6 @@ fm_to_json(struct fvwm_msg *fm)
}
}

static void
delete_pid_file(void)
{
if (pid_file == NULL)
return;

unlink(pid_file);
}

static void
set_pid_file(void)
{
char *dsp = getenv("DISPLAY");

if (dsp == NULL) {
fprintf(stderr,
"DISPLAY is not set in the environment\n");
exit(1);
}

free(pid_file);
xasprintf(&pid_file, "%s/fvwm_mfl_%s.pid", socket_basepath, dsp);
}

static void
create_pid_file(void)
{
FILE *pf;

if ((pf = fopen(pid_file, "w")) == NULL) {
fprintf(stderr, "Couldn't open %s because: %s\n", pid_file,
strerror(errno));
exit(1);
}
fprintf(pf, "%d", getpid());

if (fclose(pf) != 0) {
fprintf(stderr, "Couldn't close %s because: %s\n", pid_file,
strerror(errno));
exit(1);
}

/* Alongside sigTerm, ensure we remove the pid when exiting as well. */
atexit(delete_pid_file);
}

static int
check_pid(void)
{
FILE *pf;
int pid;

if (pid_file == NULL)
set_pid_file();

if (access(pid_file, F_OK) != 0) {
create_pid_file();
return (1);
}

if ((pf = fopen(pid_file, "r")) == NULL) {
fprintf(stderr, "Couldn't open %s for reading: %s\n", pid_file,
strerror(errno));
exit(1);
}
if (fscanf(pf, "%d", &pid) != 1) {
fprintf(stderr, "Error reading pid from %s: %s\n", pid_file,
strerror(errno));
exit(1);
};

/* Non-fatal if we can't close this file handle from reading. */
(void)fclose(pf);

if ((kill(pid, 0) == 0)) {
fprintf(stderr, "FvwmMFL is already running\n");
return (0);
}
return (1);
}


static struct fvwm_msg *
fvwm_msg_new(void)
{
Expand All @@ -246,7 +163,7 @@ static void
HandleTerminate(int fd, short what, void *arg)
{
fprintf(stderr, "%s: dying...\n", __func__);
delete_pid_file();
ThomasAdam marked this conversation as resolved.
Show resolved Hide resolved
unlink(lockfile);
unlink(socket_name);
rmdir(socket_basepath);
Copy link
Contributor

@farblos farblos Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and you probably should not just replace delete_pid_file() by unlink(lockfile), but also complement all calls to unlink(socket_name) by a call to unlink(lockfile) (plus closing the lock file handle)? There are at least two calls to unlink(socket_name) in main that do not also care about the lock file.

Or did I miss something here?

fvwmSetTerminate(fd);
Expand Down Expand Up @@ -785,7 +702,7 @@ fvwm_read(int efd, short ev, void *data)
if ((packet = ReadFvwmPacket(efd)) == NULL) {
if (debug)
fprintf(stderr, "Couldn't read from FVWM - exiting.\n");
delete_pid_file();
unlink(lockfile);
unlink(socket_name);
rmdir(socket_basepath);
exit(0);
Expand All @@ -795,7 +712,7 @@ fvwm_read(int efd, short ev, void *data)
}

void
set_socket_pathname(void)
set_pathnames(void)
{
char *mflsock_env, *display_env, *tmpdir;
const char *unrolled_path;
Expand Down Expand Up @@ -849,6 +766,31 @@ set_socket_pathname(void)

xasprintf(&socket_name, "%s/fvwm_mfl_%s.sock", socket_basepath,
display_env);

ThomasAdam marked this conversation as resolved.
Show resolved Hide resolved
xasprintf(&lockfile, "%s/fvwmmfl_lockfile_%s.lock", socket_basepath,
display_env);

}

int
lock_fvwmmfl(void)
{
struct flock l;
int fd = -1;

if ((fd = open(lockfile, O_CREAT|O_WRONLY, 0600)) == -1)
return fd;
l.l_start = 0;
l.l_type = F_WRLCK;
l.l_len = 0;
ThomasAdam marked this conversation as resolved.
Show resolved Hide resolved
l.l_whence = SEEK_SET;

if (fcntl(fd, F_SETLK, &l) == -1) {
close(fd);
return -1;
}

return fd;
}

int main(int argc, char **argv)
Expand All @@ -865,10 +807,10 @@ int main(int argc, char **argv)
return (1);
}

set_socket_pathname();
set_pathnames();

/* If we're already running... */
if (check_pid() == 0)
if ((fvwmmfl_lock_fd = lock_fvwmmfl()) == -1)
ThomasAdam marked this conversation as resolved.
Show resolved Hide resolved
return (1);

unlink(socket_name);
Expand Down
Loading