[GitHub] trafficserver pull request #1457: fix TS-4195: crash when stop trafficserver
GitHub user zizhong opened a pull request: https://github.com/apache/trafficserver/pull/1457 fix TS-4195: crash when stop trafficserver Because of `psiginfo`, `psignal` and `exit`, which call `malloc` and `free` inside `proxy_signal_handler`. ATS will crash from time to time when stop. > A signal handler can be called at any time, including during times when another call to malloc is in progress. If this happens, one of two things will occur: > > Your process will deadlock inside the signal handler, because malloc will be unable to acquire the heap lock. > Your process will corrupt its heap, because malloc does acquire the lock (or doesn't think it needs it), then proceeds to render the heap inconsistent, leading to a later crash. From [here](http://stackoverflow.com/questions/3366307/why-is-malloc-not-async-signal-safe) Tested with a script repeating starting and stopping ATS on RHEL 6.6, the current master crashes after about 2~10 times. This patch doesn't crash after 6k+ tries. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zizhong/trafficserver TS-4195 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/trafficserver/pull/1457.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1457 commit 8a9365aebd0eaa3a9fd451703996060c869fa7f5 Author: Zizhong Zhang Date: 2017-02-16T18:03:15Z fix TS-4195: double free when stop trafficserver --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1457: fix TS-4195: crash when stop trafficserver
Github user zwoop closed the pull request at: https://github.com/apache/trafficserver/pull/1457 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1457: fix TS-4195: crash when stop trafficserver
Github user bryancall commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1457#discussion_r103324828 --- Diff: proxy/Main.cc --- @@ -460,7 +460,7 @@ proxy_signal_handler(int signo, siginfo_t *info, void *ctx) shutdown_event_system = true; sleep(1); - ::exit(signo); --- End diff -- This is going to cause an issue with leak detection --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] trafficserver pull request #1457: fix TS-4195: crash when stop trafficserver
Github user zizhong commented on a diff in the pull request: https://github.com/apache/trafficserver/pull/1457#discussion_r103575599 --- Diff: proxy/Main.cc --- @@ -460,7 +460,7 @@ proxy_signal_handler(int signo, siginfo_t *info, void *ctx) shutdown_event_system = true; sleep(1); - ::exit(signo); --- End diff -- I couldn't understand why this change can cause an issue with leak detection.I read your original ticket about this line changing from `_exit` to `exit`. The reason you made that change was to fix some crashes on fedora instead of fixing leak detection issues. However, it's never okay to call `exit` in a signal handler. `exit()` is not an Async-signal-safe function, which can refer to [this man page](http://man7.org/linux/man-pages/man7/signal.7.html). Hence, we got lots of crashes when stopping or restarting ATS. Besides, @zwoop tested this patch didn't make any behavior change of leak detection tools. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
