thread_pthread.c: fix memory leak from fork loop leapfrog (v3)

Constantly forking a single-threaded process in a loop leads to
a memory leak when using POSIX timers.  This fixes the leak for
GNU/Linux systems running glibc.

v2: disarm before timer_delete
v3: ubf_timer_arm prevents double-arming

This unreverts r66291 / commit ab73ef6b7037039a05edcbf2a0c1b1108197e036
Example Linux-only reproduction may be found in:

  r66290 / commit 043047a8fd5315d98eac38ddbd04ebe8db361817

Note: FreeBSD 11.2 still leaks, I'm not sure why, yet.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@66413 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
This commit is contained in:
normal 2018-12-16 07:51:09 +00:00
parent 9fa6af24f4
commit f547d39148

View File

@ -91,12 +91,25 @@
# endif # endif
#endif #endif
enum rtimer_state {
/* alive, after timer_create: */
RTIMER_DISARM,
RTIMER_ARMING,
RTIMER_ARMED,
RTIMER_DEAD
};
#if UBF_TIMER == UBF_TIMER_POSIX #if UBF_TIMER == UBF_TIMER_POSIX
static const struct itimerspec zero;
static struct { static struct {
timer_t timerid; rb_atomic_t state; /* rtimer_state */
rb_atomic_t armed; /* 0: disarmed, 1: arming, 2: armed */
rb_pid_t owner; rb_pid_t owner;
} timer_posix; timer_t timerid;
} timer_posix = {
/* .state = */ RTIMER_DEAD,
};
#elif UBF_TIMER == UBF_TIMER_PTHREAD #elif UBF_TIMER == UBF_TIMER_PTHREAD
static void *timer_pthread_fn(void *); static void *timer_pthread_fn(void *);
static struct { static struct {
@ -1447,7 +1460,7 @@ ubf_timer_arm(rb_pid_t current) /* async signal safe */
{ {
#if UBF_TIMER == UBF_TIMER_POSIX #if UBF_TIMER == UBF_TIMER_POSIX
if ((!current || timer_posix.owner == current) && if ((!current || timer_posix.owner == current) &&
!ATOMIC_CAS(timer_posix.armed, 0, 1)) { !ATOMIC_CAS(timer_posix.state, RTIMER_DISARM, RTIMER_ARMING)) {
struct itimerspec it; struct itimerspec it;
it.it_interval.tv_sec = it.it_value.tv_sec = 0; it.it_interval.tv_sec = it.it_value.tv_sec = 0;
@ -1456,15 +1469,14 @@ ubf_timer_arm(rb_pid_t current) /* async signal safe */
if (timer_settime(timer_posix.timerid, 0, &it, 0)) if (timer_settime(timer_posix.timerid, 0, &it, 0))
rb_async_bug_errno("timer_settime (arm)", errno); rb_async_bug_errno("timer_settime (arm)", errno);
switch (ATOMIC_CAS(timer_posix.armed, 1, 2)) { switch (ATOMIC_CAS(timer_posix.state, RTIMER_ARMING, RTIMER_ARMED)) {
case 0: case RTIMER_DISARM:
/* somebody requested a disarm while we were arming */ /* somebody requested a disarm while we were arming */
it.it_interval.tv_nsec = it.it_value.tv_nsec = 0; /* may race harmlessly with ubf_timer_destroy */
if (timer_settime(timer_posix.timerid, 0, &it, 0)) (void)timer_settime(timer_posix.timerid, 0, &zero, 0);
rb_async_bug_errno("timer_settime (disarm)", errno);
case 1: return; /* success */ case RTIMER_ARMING: return; /* success */
case 2: case RTIMER_ARMED:
/* /*
* it is possible to have another thread disarm, and * it is possible to have another thread disarm, and
* a third thread arm finish re-arming before we get * a third thread arm finish re-arming before we get
@ -1472,6 +1484,10 @@ ubf_timer_arm(rb_pid_t current) /* async signal safe */
* probably unavoidable in a signal handler. * probably unavoidable in a signal handler.
*/ */
return; return;
case RTIMER_DEAD:
/* may race harmlessly with ubf_timer_destroy */
(void)timer_settime(timer_posix.timerid, 0, &zero, 0);
return;
default: default:
rb_async_bug_errno("UBF_TIMER_POSIX unknown state", ERANGE); rb_async_bug_errno("UBF_TIMER_POSIX unknown state", ERANGE);
} }
@ -1701,10 +1717,18 @@ ubf_timer_create(rb_pid_t current)
sev.sigev_notify = SIGEV_SIGNAL; sev.sigev_notify = SIGEV_SIGNAL;
sev.sigev_signo = SIGVTALRM; sev.sigev_signo = SIGVTALRM;
sev.sigev_value.sival_ptr = &timer_posix; sev.sigev_value.sival_ptr = &timer_posix;
if (!timer_create(UBF_TIMER_CLOCK, &sev, &timer_posix.timerid))
if (!timer_create(UBF_TIMER_CLOCK, &sev, &timer_posix.timerid)) {
rb_atomic_t prev = ATOMIC_EXCHANGE(timer_posix.state, RTIMER_DISARM);
if (prev != RTIMER_DEAD) {
rb_bug("timer_posix was not dead: %u\n", (unsigned)prev);
}
timer_posix.owner = current; timer_posix.owner = current;
else }
else {
rb_warn("timer_create failed: %s, signals racy", strerror(errno)); rb_warn("timer_create failed: %s, signals racy", strerror(errno));
}
#endif #endif
if (UBF_TIMER == UBF_TIMER_PTHREAD) if (UBF_TIMER == UBF_TIMER_PTHREAD)
ubf_timer_pthread_create(current); ubf_timer_pthread_create(current);
@ -1726,35 +1750,33 @@ rb_thread_create_timer_thread(void)
if (setup_communication_pipe_internal(signal_self_pipe.normal) < 0) return; if (setup_communication_pipe_internal(signal_self_pipe.normal) < 0) return;
if (setup_communication_pipe_internal(signal_self_pipe.ub_main) < 0) return; if (setup_communication_pipe_internal(signal_self_pipe.ub_main) < 0) return;
ubf_timer_create(current);
if (owner != current) { if (owner != current) {
/* validate pipe on this process */ /* validate pipe on this process */
ubf_timer_create(current);
sigwait_th = THREAD_INVALID; sigwait_th = THREAD_INVALID;
signal_self_pipe.owner_process = current; signal_self_pipe.owner_process = current;
} }
else if (UBF_TIMER == UBF_TIMER_PTHREAD) {
/* UBF_TIMER_PTHREAD needs to recreate after fork */
ubf_timer_pthread_create(current);
}
} }
static void static void
ubf_timer_disarm(void) ubf_timer_disarm(void)
{ {
#if UBF_TIMER == UBF_TIMER_POSIX #if UBF_TIMER == UBF_TIMER_POSIX
static const struct itimerspec zero; rb_atomic_t prev;
rb_atomic_t armed = ATOMIC_EXCHANGE(timer_posix.armed, 0);
if (LIKELY(armed) == 0) return; prev = ATOMIC_CAS(timer_posix.state, RTIMER_ARMED, RTIMER_DISARM);
switch (armed) { switch (prev) {
case 1: return; /* ubf_timer_arm was arming and will disarm itself */ case RTIMER_DISARM: return; /* likely */
case 2: case RTIMER_ARMING: return; /* ubf_timer_arm will disarm itself */
case RTIMER_ARMED:
if (timer_settime(timer_posix.timerid, 0, &zero, 0)) if (timer_settime(timer_posix.timerid, 0, &zero, 0))
rb_bug_errno("timer_settime (disarm)", errno); rb_bug_errno("timer_settime (disarm)", errno);
return; return;
case RTIMER_DEAD: return; /* stay dead */
default: default:
rb_bug("UBF_TIMER_POSIX bad state: %u\n", (unsigned)armed); rb_bug("UBF_TIMER_POSIX bad state: %u\n", (unsigned)prev);
} }
#elif UBF_TIMER == UBF_TIMER_PTHREAD #elif UBF_TIMER == UBF_TIMER_PTHREAD
ATOMIC_SET(timer_pthread.armed, 0); ATOMIC_SET(timer_pthread.armed, 0);
#endif #endif
@ -1763,7 +1785,21 @@ ubf_timer_disarm(void)
static void static void
ubf_timer_destroy(void) ubf_timer_destroy(void)
{ {
#if UBF_TIMER == UBF_TIMER_PTHREAD #if UBF_TIMER == UBF_TIMER_POSIX
if (timer_posix.owner == getpid()) {
/* prevent signal handler from arming: */
ATOMIC_SET(timer_posix.state, RTIMER_DEAD);
if (timer_settime(timer_posix.timerid, 0, &zero, 0))
rb_bug_errno("timer_settime (destroy)", errno);
if (timer_delete(timer_posix.timerid) < 0)
rb_sys_fail("timer_delete");
if (ATOMIC_EXCHANGE(timer_posix.state, RTIMER_DEAD) != RTIMER_DEAD) {
rb_bug("YOU KNOW I'M NOT DEAD\n");
}
}
#elif UBF_TIMER == UBF_TIMER_PTHREAD
int err; int err;
timer_pthread.owner = 0; timer_pthread.owner = 0;
@ -1774,7 +1810,6 @@ ubf_timer_destroy(void)
rb_raise(rb_eThreadError, "native_thread_join() failed (%d)", err); rb_raise(rb_eThreadError, "native_thread_join() failed (%d)", err);
} }
#endif #endif
/* no need to destroy real POSIX timers */
} }
static int static int