server icon indicating copy to clipboard operation
server copied to clipboard

MDEV-33075 [backport/2f5174e556] [10.5] Resolve server shutdown issues on macOS, Solaris, and FreeBSD

Open sitano opened this issue 1 year ago • 1 comments
trafficstars

Follow up to https://github.com/MariaDB/server/pull/3459. Differences in handle_connections_sockets() are larger in comparison to 10.11 port so I thought to make it a separate branch.


Backport of 2f5174e556cd247133aa14d7b37372ae49fe83c5: MDEV-33075 Resolve server shutdown issues on macOS, Solaris, and FreeBSD.

This commit addresses multiple server shutdown problems observed on macOS, Solaris, and FreeBSD:

  1. Corrected a non-portable assumption where socket shutdown was expected to wake up poll() with listening sockets in the main thread.

Use more robust self-pipe to wake up poll() by writing to the pipe's write end.

  1. Fixed a random crash on macOS from pthread_kill(signal_handler) when the signal_handler was detached and the thread had already exited.

Use more robust kill(getpid(), SIGTERM) to wake up the signal handler thread.

  1. Made sure, that signal handler thread always exits once abort_loop is set, and also calls my_thread_end() and clears signal_thread_in_use when exiting.

This fixes warning "1 thread did not exit" by my_global_thread_end() seen on FreeBSD/macOS when the process is terminated via signal.

Additionally, the shutdown code underwent light refactoring for better readability and maintainability:

  • Modified break_connect_loop() to no longer wait for the main thread, aligning behavior with Windows (since 10.4).
  • Removed dead code related to the unused USE_ONE_SIGNAL_HAND preprocessor constant.
  • Eliminated support for #ifndef HAVE_POLL in handle_connection_sockets This code is also dead, since 10.4

sitano avatar Aug 18 '24 20:08 sitano

@vaintroub this one is green. backport to 10.5.

sitano avatar Aug 21 '24 09:08 sitano

@vaintroub Can you please evaluate if this PR should be merged or not?

cvicentiu avatar Nov 26 '24 15:11 cvicentiu

@vaintroub Can you please evaluate if this PR should be merged or not?

This eliminates pthread_kill is favor of more reliable API, thus I think it is good, and I think should be merged. pthread_kill and the whole shutdown code was quite problematic on macOS, and IIRC, some other OSes too.

vaintroub avatar Nov 26 '24 15:11 vaintroub