v3.0.x: Fix oob_tcp tcp_component_close segfault with active listeners #7012
Conversation
oob_tcp in non-HNP mode shares libevent event_base with oob_base [1]. orte_oob_base_close calls: (1) oob_tcp component_shutdown, then (2) opal_progress_thread_finalize, then (3) oob_tcp tcp_component_close [2]. opal_progress_thread_finalize calls tracker_destructor [3] that frees the event_base [4]. If any oob_tcp event listeners are active at this time, oob_tcp will crash trying to delete them at [5] [6]. This change moves oob_tcp event listener cleanup from component_close to component_shutdown so that it happens before the event_base is freed. [1] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_listener.c#L160 [2] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/base/oob_base_frame.c#L95 [3] https://github.com/open-mpi/ompi/blob/v4.0.1/opal/runtime/opal_progress_threads.c#L232 [4] https://github.com/open-mpi/ompi/blob/v4.0.1/opal/runtime/opal_progress_threads.c#L65 [5] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_component.c#L192 [6] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_listener.c#L955 Signed-off-by: Orivej Desh <[email protected]> (cherry picked from commit 78b7e34)
|
This PR should hypothetically become moot when #7010 is cherry-picked back to the v3.0.x branch. But I'm making it anyway, just so that it's not forgotten (in case we do turn out to need it). |
|
Looks sane and self contained to me, do you know how often this segfault happens? |
|
@wckzhang This particular seg fault wasn't happening to me on the v3.0.x / v3.1.x branches, but I didn't know if this was a race condition (and therefore may not happen every time), and just hadn't been brought back to the v3.0.x / v3.1.x branches. Also, I don't know if it's now moot because of #7014...? |
|
Theoretically, I'd assume #7014 would make this patch unnecessary... Ralph had this comment in #7010 which makes me wonder if this patch actually solved the issue since apparently there was still a race condition after this patch? |
|
Best to just run a quick check - this patch should no longer be necessary. |
|
I ran the following:
I didn't get segv's in any of those cases. But #7014 is definitely a race condition. I didn't know if this PR was also addressing a race condition (and therefore I may or may not actually experience the problem). |
oob_tcp in non-HNP mode shares libevent event_base with oob_base [1].
orte_oob_base_close calls:
(1) oob_tcp component_shutdown, then
(2) opal_progress_thread_finalize, then
(3) oob_tcp tcp_component_close [2].
opal_progress_thread_finalize calls tracker_destructor [3] that frees the
event_base [4]. If any oob_tcp event listeners are active at this time, oob_tcp
will crash trying to delete them at [5] [6].
This change moves oob_tcp event listener cleanup from component_close to
component_shutdown so that it happens before the event_base is freed.
[1] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_listener.c#L160
[2] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/base/oob_base_frame.c#L95
[3] https://github.com/open-mpi/ompi/blob/v4.0.1/opal/runtime/opal_progress_threads.c#L232
[4] https://github.com/open-mpi/ompi/blob/v4.0.1/opal/runtime/opal_progress_threads.c#L65
[5] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_component.c#L192
[6] https://github.com/open-mpi/ompi/blob/v4.0.1/orte/mca/oob/tcp/oob_tcp_listener.c#L955
Signed-off-by: Orivej Desh [email protected]
(cherry picked from commit 78b7e34)