Move the Event Hubs link and session creation outside the client locks

オープン
  • 何か問題がありますか？
#7,286 コメント 0 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@j7nw4r がすでに取り組んでいます。

2026年8月5日 から。

評価

この issue はまだ評価されていません。

説明

Event Hubs

Summary

ProducerClient::EnsureSender keeps m_sendersLock across the link creation and MessageSender::Open. Open sends data to the service and waits for the answer. The first send to one partition must therefore wait for the link setup of every other partition. This also puts a lock across a call that the AMQP layer identifies as a deadlock risk.

Parent: #7284. Severity tags follow the key in #7252. The code anchors refer to commit 2de7c583e.

Motivation

EnsureSender takes the lock at src/producer_client.cpp:229. It keeps the lock through GetSession(partitionId).CreateMessageSender(targetUrl, senderOptions) and sender.Open(context). It writes to the map at src/producer_client.cpp:256. Open does the AMQP attach handshake, so the lock stays locked for a full network round trip.

EnsureSession and EnsureConnection have the same design under m_sessionsLock. That mutex is a std::recursive_mutex (inc/azure/messaging/eventhubs/producer_client.hpp:197). The recursion is necessary only because EnsureSession calls EnsureConnection while it keeps the lock. Move the creation outside the lock, and the recursion stops. The mutex can then be a plain mutex.

The AMQP layer identifies a related risk. MessageSenderImpl::Open registers the link with the global polling thread through AddPollable (azure-core-amqp/src/impl/uamqp/amqp/message_sender.cpp:290,340). The note at azure-core-amqp/src/common/global_state.cpp:159 says that the caller must not keep a connection lock or a link lock when it calls AddPollable. The polling thread takes the connection lock, and the two threads then deadlock.

m_sendersLock is an Event Hubs lock, not a link lock. The deadlock in that note therefore does not occur as written. The call stack does nest Event Hubs locks across that boundary. The transport authors identified this pattern as a risk.

Proposal

  • Change EnsureSender to a double-checked write (src/producer_client.cpp:225-257). [GA blocker]
  • Change EnsureSession and EnsureConnection in the same way (src/producer_client.cpp:197-224). [GA blocker]
  • Change the equivalent ConsumerClient methods in the same way (src/consumer_client.cpp:120-165). [GA blocker]
  • Change m_sessionsLock from std::recursive_mutex to std::mutex after the recursion stops (inc/azure/messaging/eventhubs/producer_client.hpp:197, inc/azure/messaging/eventhubs/consumer_client.hpp:210). [GA quality bar]

The double-checked write has four steps. Read the map under the lock. Release the lock. Build the link and call Open with no lock. Take the lock again and write to the map after a second read.

Two threads can create a link for the same partition at the same time. One thread wins. The other thread must close its extra link before it returns the handle of the winner. Do not discard the extra link, because that leaks a link.

Validation

  • No lock stays locked across Open, CreateMessageSender, or any other call that uses the network. A code review of the four call sites shows this.
  • A test that opens links for N partitions at the same time takes about the time of one link setup.
  • The thread sanitizer reports no race for the double-checked write.
  • Two threads that create the same partition at the same time leak no link.
  • m_sessionsLock is a plain std::mutex. The tests pass.
主要言語
C++
スター
206
フォーク
178
平均マージ
2日 15時間
マージ済み PR（30日）
33

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

Azure/azure-sdk-for-cpp のほかの issue

Azure/azure-sdk-for-cpp の issue をすべて見る

似ている issue

C++ の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。