From 736c5f26e57760325e9023fa3e5c2beb1d581483 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 31 Mar 2012 13:34:42 +0200 Subject: [PATCH 1/2] s3-aio-fork: Fix aio_suspend event hierarchy We end up here multiple times. There's no real point putting the events into the child struct, at the end of this routine we need to free them anyway. --- source3/modules/vfs_aio_fork.c | 18 ++++++++---------- 1 files changed, 8 insertions(+), 10 deletions(-) diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c index a66d39d..9a978dd 100644 --- a/source3/modules/vfs_aio_fork.c +++ b/source3/modules/vfs_aio_fork.c @@ -827,6 +827,8 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle, */ for (child = children->children; child != NULL; child = child->next) { + struct tevent_fd *event; + if (child->aiocb == NULL) { continue; } @@ -841,16 +843,12 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle, continue; } - /* We're never using this event on the - * main event context again... */ - TALLOC_FREE(child->sock_event); - - child->sock_event = event_add_fd(ev, - child, - child->sockfd, - EVENT_FD_READ, - handle_aio_completion, - child); + event = event_add_fd(ev, + frame, + child->sockfd, + EVENT_FD_READ, + handle_aio_completion, + child); while (1) { if (tevent_loop_once(ev) == -1) { -- 1.7.3.4 From 3374dbd99c819d763b4469366c3591638f2605d3 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 31 Mar 2012 13:37:20 +0200 Subject: [PATCH 2/2] s3-aio-fork: Fix a segfault in vfs_aio_fork aio_suspend does not signal the main process with a signal, it just waits. The aio_fork module does not use the signal at all, it directly calls back into the main smbd by calling smbd_aio_complete_aio_ex. This is an abstraction violation, but the alternative would have been to use signals where they are not needed. However, in wait_for_aio_completion this bites us: With aio_fork we call handle_aio_completed twice on the same aio_ex struct: Once from the call to handle_aio_completion within the aio_fork module and once from the code in wait_for_aio_completion. This patch fixes it in a pretty bad way by introducing flag variables and more state. But the mid-term plan is to replace the posix aio calls from the vfs and do pread_send/recv and pwrite_send/recv at the vfs layer, so this will significantly change anyway. Thanks to Kirill Malkin for reporting this crash! --- source3/modules/vfs_aio_fork.c | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/source3/modules/vfs_aio_fork.c b/source3/modules/vfs_aio_fork.c index 9a978dd..187166c 100644 --- a/source3/modules/vfs_aio_fork.c +++ b/source3/modules/vfs_aio_fork.c @@ -101,6 +101,8 @@ struct aio_child { bool dont_delete; /* Marked as in use since last cleanup */ bool cancelled; bool read_cmd; + bool called_from_suspend; + bool completion_done; }; struct aio_child_list { @@ -432,6 +434,10 @@ static void handle_aio_completion(struct event_context *event_ctx, child->retval.size); } + if (child->called_from_suspend) { + child->completion_done = true; + return; + } aio_ex = (struct aio_extra *)child->aiocb->aio_sigevent.sigev_value.sival_ptr; smbd_aio_complete_aio_ex(aio_ex); TALLOC_FREE(aio_ex); @@ -850,7 +856,9 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle, handle_aio_completion, child); - while (1) { + child->called_from_suspend = true; + + while (!child->completion_done) { if (tevent_loop_once(ev) == -1) { goto out; } @@ -859,12 +867,6 @@ static int aio_fork_suspend(struct vfs_handle_struct *handle, errno = EAGAIN; goto out; } - - /* We set child->aiocb to NULL in our hooked - * AIO_RETURN(). */ - if (child->aiocb == NULL) { - break; - } } } } -- 1.7.3.4