Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock when closing Broker in brokerProducer #2133

Merged
merged 5 commits into from
Feb 13, 2022

Commits on Feb 8, 2022

  1. Fix deadlock when closing Broker in brokerProducer

    - add unit test to reproduce the deadlock by simulating a network error
    - document possible deadlock when closing the Broker from an AsyncProduce
      callback when handling a response error
    - add closeBroker goroutine and channel to asynchronously close a Broker
      once
    - reuse the stopchan channel to signal that the closeBroker goroutine is
      done
    - update TestBrokerProducerShutdown to check goroutine leak by closing
      the input vs the stopchan channel
    - fixes IBM#2129
    slaunay committed Feb 8, 2022
    Configuration menu
    Copy the full SHA
    59dd565 View commit details
    Browse the repository at this point in the history

Commits on Feb 9, 2022

  1. Address possible data race with the responses chan

    WARNING: DATA RACE
    Write at 0x00c0003421f0 by goroutine 71:
      runtime.closechan()
          runtime/chan.go:355 +0x0
      github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1()
          github.com/Shopify/sarama/async_producer.go:725 +0x1c4
      github.com/Shopify/sarama.withRecover()
          github.com/Shopify/sarama/utils.go:43 +0x74
      github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer·dwrap·15()
          github.com/Shopify/sarama/async_producer.go:695 +0x39
    
    Previous read at 0x00c0003421f0 by goroutine 58:
      runtime.chansend()
          runtime/chan.go:158 +0x0
      github.com/Shopify/sarama.(*asyncProducer).newBrokerProducer.func1.1.1()
          github.com/Shopify/sarama/async_producer.go:702 +0x125
      github.com/Shopify/sarama.(*Broker).AsyncProduce.func1()
          github.com/Shopify/sarama/broker.go:408 +0x1a9
      github.com/Shopify/sarama.(*responsePromise).handle()
          github.com/Shopify/sarama/broker.go:132 +0x1b8
      github.com/Shopify/sarama.(*Broker).responseReceiver()
          github.com/Shopify/sarama/broker.go:1040 +0x124
      github.com/Shopify/sarama.(*Broker).responseReceiver-fm()
          github.com/Shopify/sarama/broker.go:1032 +0x39
      github.com/Shopify/sarama.withRecover()
          github.com/Shopify/sarama/utils.go:43 +0x74
      github.com/Shopify/sarama.(*Broker).Open.func1·dwrap·22()
          github.com/Shopify/sarama/broker.go:244 +0x39
    slaunay committed Feb 9, 2022
    Configuration menu
    Copy the full SHA
    6c70a6c View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    8f92872 View commit details
    Browse the repository at this point in the history

Commits on Feb 11, 2022

  1. Keep closing the broker synchronously but buffer pending responses

    Closing the broker asynchronously fixes the deadlock but leads to a
    race condition between opening the broker in client updateLeader.
    This might result in a closed broker used by the new brokerProducer
    and all produce requests will fail with ErrNotConnected.
    slaunay committed Feb 11, 2022
    Configuration menu
    Copy the full SHA
    f1bc44e View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    85a2d3c View commit details
    Browse the repository at this point in the history