| Message ID | 1375831766-3444-1-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/07/13 01:29, Amos Kong wrote: > We register exit clean function by atexit(), > but alarm_timer is NULL here. If exit is caused > between atexit() and alarm_timer assignment, > real timer can't be cleaned. That's correct in general, but I don't see how it could happen in the code being patched. pthread_atfork() won't call exit(). Thanks, Laszlo > So move alarm_timer > assignment before atexit(). > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > qemu-timer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qemu-timer.c b/qemu-timer.c > index b2d95e2..9490105 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -767,11 +767,11 @@ int init_timer_alarm(void) > goto fail; > } > > + alarm_timer = t; > atexit(quit_timers); > #ifdef CONFIG_POSIX > pthread_atfork(NULL, NULL, reinit_timers); > #endif > - alarm_timer = t; > return 0; > > fail: >
On Wed, Aug 07, 2013 at 08:39:19AM +0200, Laszlo Ersek wrote: > On 08/07/13 01:29, Amos Kong wrote: > > We register exit clean function by atexit(), > > but alarm_timer is NULL here. If exit is caused > > between atexit() and alarm_timer assignment, > > real timer can't be cleaned. > > That's correct in general, but I don't see how it could happen in the > code being patched. pthread_atfork() won't call exit(). Agreed. I can remember thinking about this when reading the code and deciding not to bother changing it. Since the patch is on the list though, we might as well apply it. The only thing I suggest changing is to note that this is currently not a bug, just a clean-up. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/qemu-timer.c b/qemu-timer.c index b2d95e2..9490105 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -767,11 +767,11 @@ int init_timer_alarm(void) goto fail; } + alarm_timer = t; atexit(quit_timers); #ifdef CONFIG_POSIX pthread_atfork(NULL, NULL, reinit_timers); #endif - alarm_timer = t; return 0; fail:
We register exit clean function by atexit(), but alarm_timer is NULL here. If exit is caused between atexit() and alarm_timer assignment, real timer can't be cleaned. So move alarm_timer assignment before atexit(). Signed-off-by: Amos Kong <akong@redhat.com> --- qemu-timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)