Skip to content

Introduce a special FPM event pid callback #10510

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

Closed
wants to merge 1 commit into from
Closed

Introduce a special FPM event pid callback #10510

Show file tree
Hide file tree
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
25 changes: 21 additions & 4 deletions sapi/fpm/fpm/fpm_events.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,14 @@ void fpm_event_loop(int err) /* {{{ */

void fpm_event_fire(struct fpm_event_s *ev) /* {{{ */
{
if (!ev || !ev->callback) {
if (!ev || !ev->ptr_callback) {
Copy link

Choose a reason for hiding this comment

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

ptr_callback or pid_callback should be checked below, after dispatch based on pid.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's union so the same memory is used so if one is NULL, then the other one is NULL as well.

Copy link

Choose a reason for hiding this comment

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

Of course this is true, but this is not explicitly apparent when reading this code and could be easily forgotten if the structure is modified.

return;
}

(*ev->callback)( (struct fpm_event_s *) ev, ev->which, ev->arg);
if (ev->pid) {
(*ev->pid_callback)( (struct fpm_event_s *) ev, ev->which, ev->pid);
} else {
(*ev->ptr_callback)( (struct fpm_event_s *) ev, ev->which, ev->arg);
}
}
/* }}} */

Expand All @@ -495,13 +498,27 @@ int fpm_event_set(struct fpm_event_s *ev, int fd, int flags, void (*callback)(st
}
memset(ev, 0, sizeof(struct fpm_event_s));
ev->fd = fd;
ev->callback = callback;
ev->ptr_callback = callback;
ev->arg = arg;
ev->flags = flags;
return 0;
}
/* }}} */

int fpm_event_set_with_pid(struct fpm_event_s *ev, int fd, int flags, void (*callback)(struct fpm_event_s *, short, pid_t pid), pid_t pid) /* {{{ */
{
if (!ev || !callback || fd < -1) {
return -1;
}
memset(ev, 0, sizeof(struct fpm_event_s));
ev->fd = fd;
ev->pid_callback = callback;
ev->pid = pid;
ev->flags = flags;
return 0;
}
/* }}} */

int fpm_event_add(struct fpm_event_s *ev, unsigned long int frequency) /* {{{ */
{
struct timeval now;
Expand Down
7 changes: 6 additions & 1 deletion sapi/fpm/fpm/fpm_events.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@ struct fpm_event_s {
int fd; /* not set with FPM_EV_TIMEOUT */
struct timeval timeout; /* next time to trigger */
struct timeval frequency;
void (*callback)(struct fpm_event_s *, short, void *);
union {
void (*ptr_callback)(struct fpm_event_s *, short, void *);
void (*pid_callback)(struct fpm_event_s *, short, pid_t pid);
};
Copy link

Choose a reason for hiding this comment

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

Propose to document in a comment how the proper field should be selected (pid != 0).

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah will add a comment if this is confirmed working :)

void *arg;
int flags;
int index; /* index of the fd in the ufds array */
short which; /* type of event */
pid_t pid;
};

typedef struct fpm_event_queue_s {
Expand All @@ -41,6 +45,7 @@ void fpm_event_loop(int err);
void fpm_event_fire(struct fpm_event_s *ev);
int fpm_event_init_main(void);
int fpm_event_set(struct fpm_event_s *ev, int fd, int flags, void (*callback)(struct fpm_event_s *, short, void *), void *arg);
int fpm_event_set_with_pid(struct fpm_event_s *ev, int fd, int flags, void (*callback)(struct fpm_event_s *, short, pid_t pid), pid_t pid);
int fpm_event_add(struct fpm_event_s *ev, unsigned long int timeout);
int fpm_event_del(struct fpm_event_s *ev);
int fpm_event_pre_init(char *machanism);
Expand Down
10 changes: 5 additions & 5 deletions sapi/fpm/fpm/fpm_stdio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ int fpm_stdio_flush_child(void)
return write(STDERR_FILENO, FPM_STDIO_CMD_FLUSH, sizeof(FPM_STDIO_CMD_FLUSH));
}

static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, pid_t pid) /* {{{ */
{
static const int max_buf_size = 1024;
int fd = ev->fd;
Expand All @@ -178,10 +178,10 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
int read_fail = 0, create_log_stream;
struct zlog_stream *log_stream;

if (!arg) {
if (!pid) {
return;
}
child = fpm_child_find((intptr_t) arg);
child = fpm_child_find(pid);
if (!child) {
return;
}
Expand Down Expand Up @@ -330,10 +330,10 @@ int fpm_stdio_parent_use_pipes(struct fpm_child_s *child) /* {{{ */
child->fd_stdout = fd_stdout[0];
child->fd_stderr = fd_stderr[0];

fpm_event_set(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
fpm_event_set_with_pid(&child->ev_stdout, child->fd_stdout, FPM_EV_READ, fpm_stdio_child_said, child->pid);
fpm_event_add(&child->ev_stdout, 0);

fpm_event_set(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, (void *) (intptr_t) child->pid);
fpm_event_set_with_pid(&child->ev_stderr, child->fd_stderr, FPM_EV_READ, fpm_stdio_child_said, child->pid);
fpm_event_add(&child->ev_stderr, 0);
return 0;
}
Expand Down