[patches] Re: [commits] [tarantool] 01/01: log: Fix log rotation is not async signal safe

[Konstantin Osipov]

* IlyaMarkovMipt <markovilya197 at gmail.com> [18/01/24 13:34]:
> commit a254d2b1cd8fb316ac3ad3ae36d59686d86cc8e8
> Author: IlyaMarkovMipt <markovilya197 at gmail.com>
> AuthorDate: Tue Jan 23 16:23:29 2018 +0300
> 
>     log: Fix log rotation is not async signal safe
>     
>     * Refactor signal handling with ev_signal.
>     * Add another signal watcher in main.
>     
>     We claim multiple loggers non-thread-safe,
>     so concurrency in logrotate signal handler
>     should not be considered.

I'd be very cautious to make log rotation non-asynchronous. 
If we run out of disk space, and the logger goes to a file,
write() can be completely stuck until there is available disk
space. In this case the entire event loop will be stuck, and
the system will sit in a dead lock.
>     
>     Relates #3015
> ---
>  src/lua/log.lua | 8 ++++++--
>  src/main.cc     | 3 ++-
>  src/say.c       | 7 ++++---
>  src/say.h       | 5 ++++-
>  4 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/src/lua/log.lua b/src/lua/log.lua
> index b90eb88..0ac0e8f 100644
> --- a/src/lua/log.lua
> +++ b/src/lua/log.lua
> @@ -12,7 +12,11 @@ ffi.cdef[[
>  
>  
>      extern sayfunc_t _say;
> -    extern void say_logrotate(int);
> +    extern struct ev_loop;
> +    extern struct ev_signal;
> +
> +    extern void
> +    say_logrotate(struct ev_loop *, struct ev_signal *, int);
>  
>      enum say_level {
>          S_FATAL,
> @@ -104,7 +108,7 @@ local function say_closure(lvl)
>  end
>  
>  local function log_rotate()
> -    ffi.C.say_logrotate(0)
> +    ffi.C.say_logrotate(nil, nil, 0)
>  end
>  
>  local function log_level(level)
> diff --git a/src/main.cc b/src/main.cc
> index ec06103..1682bae 100644
> --- a/src/main.cc
> +++ b/src/main.cc
> @@ -83,7 +83,7 @@ static char *pid_file = NULL;
>  static char **main_argv;
>  static int main_argc;
>  /** Signals handled after start as part of the event loop. */
> -static ev_signal ev_sigs[5];
> +static ev_signal ev_sigs[6];
>  static const int ev_sig_count = sizeof(ev_sigs)/sizeof(*ev_sigs);
>  
>  static double start_time;
> @@ -335,6 +335,7 @@ signal_init(void)
>  	ev_signal_init(&ev_sigs[2], signal_cb, SIGTERM);
>  	ev_signal_init(&ev_sigs[3], signal_cb, SIGHUP);
>  	ev_signal_init(&ev_sigs[4], signal_sigwinch_cb, SIGWINCH);
> +	ev_signal_init(&ev_sigs[5], say_logrotate, SIGHUP);
>  	for (int i = 0; i < ev_sig_count; i++)
>  		ev_signal_start(loop(), &ev_sigs[i]);
>  
> diff --git a/src/say.c b/src/say.c
> index 65c1872..394e6b3 100644
> --- a/src/say.c
> +++ b/src/say.c
> @@ -269,9 +269,11 @@ log_rotate(const struct log *log)
>  }
>  
>  void
> -say_logrotate(int signo)
> +say_logrotate(struct ev_loop *loop, struct ev_signal *w, int revents)
>  {
> -	(void) signo;
> +	(void) loop;
> +	(void) w;
> +	(void) revents;
>  	int saved_errno = errno;
>  	struct log *log;
>  	rlist_foreach_entry(log, &log_rotate_list, in_log_list) {
> @@ -535,7 +537,6 @@ say_logger_init(const char *init_str, int level, int nonblock,
>  	say_set_log_level(level);
>  	log_background = background;
>  	log_pid = log_default->pid;
> -	signal(SIGHUP, say_logrotate);
>  	say_set_log_format(say_format_by_name(format));
>  
>  	if (background) {
> diff --git a/src/say.h b/src/say.h
> index 1b63229..46e6976 100644
> --- a/src/say.h
> +++ b/src/say.h
> @@ -185,8 +185,11 @@ say_set_log_format(enum say_format format);
>  enum say_format
>  say_format_by_name(const char *format);
>  
> +struct ev_loop;
> +struct ev_signal;
> +
>  void
> -say_logrotate(int /* signo */);
> +say_logrotate(struct ev_loop *, struct ev_signal *, int /* revents */);
>  
>  /** Init default logger. */
>  void
> 
> -- 
> To stop receiving notification emails like this one, please contact
> the administrator of this repository.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.org - www.twitter.com/kostja_osipov