From: Jérémie Galarneau Date: Tue, 21 Apr 2020 21:08:10 +0000 (-0400) Subject: Fix: sessiond: sessiond and agent deadlock on destroy X-Git-Tag: v2.13.0-rc1~659 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=733c916573fb96d14ff36cc0a034c5b1ef7ef167;hp=733c916573fb96d14ff36cc0a034c5b1ef7ef167 Fix: sessiond: sessiond and agent deadlock on destroy Observed issue -------------- While running the out-of-tree java agent tests [1], the session daemon and agent often end up in a deadlock. Attaching gdb to the session daemon, we can see that two threads are blocked in an intriguing state. Thread 13 (Thread 0x7f89027fc700 (LWP 9636)): #0 0x00007f891e81a4cf in __lll_lock_wait () from /usr/lib/libpthread.so.0 #1 0x00007f891e812e03 in pthread_mutex_lock () from /usr/lib/libpthread.so.0 #2 0x000055637f1fbd92 in session_lock_list () at session.c:156 #3 0x000055637f25dc47 in update_agent_app (app=0x7f88ec003480) at agent-thread.c:56 #4 0x000055637f25ec0a in thread_agent_management (data=0x556380cd2400) at agent-thread.c:426 #5 0x000055637f22fb3a in launch_thread (data=0x556380cd24a0) at thread.c:65 #6 0x00007f891e81046f in start_thread () from /usr/lib/libpthread.so.0 #7 0x00007f891e7203d3 in clone () from /usr/lib/libc.so.6 Thread 8 (Thread 0x7f8919309700 (LWP 9631)): #0 0x00007f891e81b44d in recvmsg () from /usr/lib/libpthread.so.0 #1 0x000055637f267847 in lttcomm_recvmsg_inet_sock (sock=0x7f88ec0033c0, buf=0x7f89192f5d5c, len=4, flags=0) at inet.c:367 #2 0x000055637f2146c6 in recv_reply (sock=0x7f88ec0033c0, buf=0x7f89192f5d5c, size=4) at agent.c:275 #3 0x000055637f215202 in app_context_op (app=0x7f88ec003400, ctx=0x7f8908020900, cmd=AGENT_CMD_APP_CTX_DISABLE) at agent.c:552 #4 0x000055637f215c2d in disable_context (ctx=0x7f8908020900, domain=LTTNG_DOMAIN_JUL) at agent.c:841 #5 0x000055637f217480 in agent_destroy (agt=0x7f890801dc20) at agent.c:1326 #6 0x000055637f243448 in trace_ust_destroy_session (session=0x7f8908004010) at trace-ust.c:1408 #7 0x000055637f1fd775 in session_release (ref=0x7f8908001e70) at session.c:873 #8 0x000055637f1fb9ac in urcu_ref_put (ref=0x7f8908001e70, release=0x55637f1fd62a ) at /usr/include/urcu/ref.h:68 #9 0x000055637f1fdad2 in session_put (session=0x7f8908000d10) at session.c:942 #10 0x000055637f2369e6 in process_client_msg (cmd_ctx=0x7f890800e6e0, sock=0x7f8919308560, sock_error=0x7f8919308564) at client.c:2102 #11 0x000055637f2375ab in thread_manage_clients (data=0x556380cd1840) at client.c:2347 #12 0x000055637f22fb3a in launch_thread (data=0x556380cd18b0) at thread.c:65 #13 0x00007f891e81046f in start_thread () from /usr/lib/libpthread.so.0 #14 0x00007f891e7203d3 in clone () from /usr/lib/libc.so.6 T8 is holding session list lock while the cmd_destroy_session command is being processed. More specifically, it is attempting to destroy an "agent_context" by communicating with an "agent" application. Meanwhile, T13 is still registering that same "agent" application. Cause ----- The deadlock itself is pretty simple to understand. The "agent thread" (T13) has the responsability of accepting new agent application connections. When such a connection occurs, the thread creates a new `agent_app` instance and sends the current sessions' configuration (i.e. their event rules and contexts) to the agent application. When that "update" is complete, a "registration done" message is sent to the new agent application. From the stacktrace above, we can see that T13 is attempting to update the agent application with its initial configuration, but it is blocked on the acquisition of the session list lock. The application's agent is also blocked since it is waiting for the "registration done" message before allowing tracing to proceed (not shown here, but seen in the test logs). Meanwhile, T8 is holding the session list lock while destroying a session. This is expected as all client commands are executed with this lock held. It is, amongst other reasons, used to serialize changes to the sessions' configuration and configuration updates sent to the tracers (i.e. because new apps appear or to keep existing tracers in sync with the users' session configuration). The question becomes: why is T8 tearing down an application that is not yet registered? First, inspecting `agent_app` immediately shows that this structure has no built-in synchronization mechanism. Therefore, the fact that two threads are accessing it at the same time raises a big red flag. Speculating on the intentions of the original design, my intuition is that the "agent_management" thread's role is limited to instantiating an `agent_app` and synchronizing it with the various sessions' configuration. Once that synchronization is performed, the agent application should be published and never accessed again by the "agent thread". Configuration updates (i.e. new event rules, contexts) are then sent synchronously as they are requested by a client in the context of the client thread. Those updates are performed while holding the session list lock. Hence, there is only one thread that should manipulate the agent application at any given time making an explicit `agent_app` lock unnecessary. Overall, this would echo what is done when a 'user space tracer' application registers to the session daemon (see dispatch.c:368). Evidently this isn't what is happening here. The agent thread creates the `agent_app`, publishes it, and then performs an "agent app update" (sending the configuration) while holding the session list lock. This means that there is a window where an agent application is visible to the other threads, yet has not been properly registered. Solution -------- The acquisition of the session list lock is moved outside of update_agent_app() to allow the "agent thread" to hold the session list lock during the "configuration update" phase of the agent application registration. Essentially, the sequence of operation changes from: - Agent tcp connection established - call handle_registration() - agent version check - allocation of agent_app instance - new agent_add is published through the global agent_apps_ht_by_sock hashtable *** it is now reachable by all other threads without any form of exclusivity synchronization. *** - update_agent_app - acquire session list lock - iterate over sessions - send configuration - release session list lock - send registration done to: - Agent tcp connection established - call accept_agent_registration() - agent version check - allocation of agent_app instance - acquire session list lock - update_agent_app - iterate over sessions - send configuration - send registration done - new agent_add is published through the global agent_apps_ht_by_sock hashtable - release session list lock Links ----- [1] https://github.com/lttng/lttng-ust-java-tests Signed-off-by: Jérémie Galarneau Change-Id: Ia34c5ad81ed3936acbca756b425423e0cb8dbddf ---