[Bug] Multiple self-deadlocks in CycloneDDS caused by recursive locking of non-recursive mutexes

[hanqing2025]

Summary

Dear CycloneDDS maintainers,

Thank you for your work on the project. During our analysis, we found 8 self-deadlock cases in CycloneDDS that appear to share the same root cause.

In the observed cases, a thread acquires a non-recursive mutex and later re-enters a path that attempts to acquire the same mutex again before the first acquisition is released, resulting in self-deadlock.

Concrete example

One concrete case is gv->sendq_running_lock.

CycloneDDS acquires this lock in:

• src/core/ddsc/src/dds_writer.c:459
• src/core/ddsi/src/ddsi_init.c:1921

In the self-loop we observed, the path is rooted at dds_delete(). During deletion and cleanup, CycloneDDS enters ddsi_fini(), which acquires gv->sendq_running_lock at src/core/ddsi/src/ddsi_init.c:1921. We observed nested deletion/finalization paths in which the same thread re-enters cleanup before the first acquisition is released, and then tries to acquire gv->sendq_running_lock again.

A representative captured stack for this re-entry is:

dds_delete -> dds_delete_impl -> dds_delete_impl_pinned -> really_delete_pinned_closed_locked -> dds_entity_deriver_delete -> dds_domain_free -> ddsi_fini -> joinleave_spdp_defmcip -> ddsi_addrset_forall -> ddsi_leave_mc -> ddsrt_mutex_lock

Because gv->sendq_running_lock is non-recursive, this second acquisition attempt deadlocks the current thread on its own lock.

Affected revision

Latest upstream revision analyzed:

• be3ecb81f4d6f0664d92fbbc0a2f8edb553bb4bf

Earlier releases also appear to be affected, but we have not yet determined the exact introduction point.

Affected sites

1. entity->qos_lock

   • src/core/ddsc/src/dds_serdata_builtintopic.c:219

2. gv->sendq_running_lock

   • src/core/ddsc/src/dds_writer.c:459
   • src/core/ddsi/src/ddsi_init.c:1921

3. lowr->wr.e.lock

   • src/core/ddsi/src/ddsi_endpoint.c:1320

4. observed->m_observers_lock

   • src/core/ddsc/src/dds_entity.c:1394
   • src/core/ddsc/src/dds_entity.c:1418

5. parent->m_mutex

   • src/core/ddsc/src/dds_entity.c:335
   • src/core/ddsc/src/dds_entity.c:346

6. pp->e.lock

   • src/core/ddsi/src/ddsi_participant.c:754
   • src/core/ddsi/src/ddsi_participant.c:991

7. rd->m_entity.m_observers_lock

   • src/core/ddsc/src/dds_reader.c:225

8. thread_states.lock

   • src/core/ddsi/src/ddsi_thread.c:257
   • src/core/ddsi/src/ddsi_thread.c:261

Impact

Any affected path can hang the calling thread and potentially the whole CycloneDDS process, resulting in denial of service.

Suggested direction

The fix likely needs to be the same at all sites:

• avoid calling re-entrant helpers while holding these mutexes, or
• restructure the call paths so the same mutex cannot be acquired twice by the same thread, or
• only use recursive mutexes where re-entrancy is explicitly intended and proven safe

Reference

• Upstream tree: https://github.com/eclipse-cyclonedds/cyclonedds/tree/be3ecb81f4d6f0664d92fbbc0a2f8edb553bb4bf

[eboasson]

Thank you for the report. I do wonder: did you actually observe this deadlock? Because the concrete case of the two locations of gv->sendq_running_lock can never be happen. It is impossible to create a writer (the one at dds_writer.c:459) until after the stack has been initialized, and it is impossible to stop the stack (the one at ddsi_init.c:1921) while there is still a writer in existence. This suggests an insufficiently precise static analysis.

That's not to say that there are no deadlocks. Neither do I want to suggest that I am not grateful to anyone who puts the effort in to analyze the code and report issues.

[hanqing2025]

Thank you for the detailed explanation. we did observe a real deadlock. It was found by
our detection tool by freely composing public APIs and then running the generated
harness. The concrete reproduced case is different from the
gv->sendq_running_lock example: it is a same-entity listener reentrancy
deadlock involving dds_set_listener().

Summary

Calling dds_set_listener(entity, NULL) from a listener callback that is currently being invoked for the same entity can deadlock. I reproduced this with a DataWriter on_publication_matched callback: the callback is invoked while the writer has m_cb_pending_count > 0, and dds_set_listener() waits for that counter to become zero. Because the current callback cannot finish until dds_set_listener() returns, the thread waits for itself.

This is different from the earlier gv->sendq_running_lock example. This report uses only public API calls and has a reproducer.

Affected version tested

Tested on commit be3ecb81.

Reproducer

The reproducer is in:

• cyclonedds/listener_deadlock_poc/DeadlockData.idl(can't upload idl extention, maybe you should change this manually)
• cyclonedds/listener_deadlock_poc/listener_set_deadlock.c
• cyclonedds/listener_deadlock_poc/CMakeLists.txt

The essential callback is:

static void on_publication_matched (
  dds_entity_t writer,
  const dds_publication_matched_status_t status,
  void *arg)
{
  (void) arg;
  printf ("callback entered ... calling dds_set_listener(writer, NULL)\n");
  fflush (stdout);
  dds_set_listener (writer, NULL);
  printf ("UNEXPECTED: dds_set_listener returned\n");
}

Build and run:

cmake -S cyclonedds/listener_deadlock_poc -B /tmp/cyclonedds-listener-deadlock-poc-build \
  -DCMAKE_PREFIX_PATH=/path/to/cyclonedds/install
cmake --build /tmp/cyclonedds-listener-deadlock-poc-build -j2
timeout 6s /tmp/cyclonedds-listener-deadlock-poc-build/listener_set_deadlock
echo $?

Observed output:

callback entered: current_count_change=1, calling dds_set_listener(writer, NULL)
124

The process times out. The callback never returns from dds_set_listener().

Why it deadlocks

In dds_writer_status_cb():

• dds_writer.c:126 locks wr->m_entity.m_observers_lock
• dds_writer.c:127 increments m_cb_pending_count
• dds_writer.c:130 increments m_cb_count
• dds_writer.c:145 invokes status_cb_publication_matched()

The status callback macro releases m_observers_lock before calling user code:

• dds__entity.h:146 unlocks m_observers_lock
• dds__entity.h:147 invokes the listener callback
• dds__entity.h:148 would lock it again after the callback returns

However, the counters remain nonzero while user code runs. If that user callback calls dds_set_listener() on the same writer:

• dds_entity.c:1053 locks the same entity's m_observers_lock
• dds_entity.c:1054-1055 waits while m_cb_pending_count > 0

At that point m_cb_pending_count is the count for the callback currently executing on the same call stack. It can only be decremented after the callback returns to dds_writer_status_cb() at dds_writer.c:159-160, but the callback is blocked in dds_set_listener().

Backtrace

The hung thread is blocked in dds_set_listener() waiting on m_observers_cond:

#6  dds_set_listener (...) at src/core/ddsc/src/dds_entity.c:1055
#7  on_publication_matched (...) at listener_set_deadlock.c:43
#8  status_cb_publication_matched_invoke (...) at src/core/ddsc/src/dds_writer.c:104
#10 dds_writer_status_cb (...) at src/core/ddsc/src/dds_writer.c:145
#11 ddsi_writer_add_local_connection (...) at src/core/ddsi/src/ddsi_endpoint_match.c:891
#15 dds_create_reader_int (...) at src/core/ddsc/src/dds_reader.c:660
#16 dds_create_reader (...) at src/core/ddsc/src/dds_reader.c:832
#17 main (...) at listener_set_deadlock.c:71

Expected behavior

If changing/removing a listener from within its own callback is supported, dds_set_listener() should not wait for the callback currently executing on the same thread. If it is unsupported, the API should fail cleanly or document the restriction; it should not block indefinitely.

CMakeLists.txt
listener_set_deadlock.c

DeadlockData.txt