[IMPALA-2642] Potential deadlock in statestore error path

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: Impala 2.3.0
• Fix Version/s: Impala 2.12.0
• Component/s: Distributed Exec
• Labels: hang statestore
• Target Version: Product Backlog
• Assignee: Zoram Thanga
• Reporter: Henry Robinson
• Created: 06/Nov/15 00:31
• Updated: 06/Mar/18 18:34
• Resolved: 06/Mar/18 18:34

Description

I just noticed this while reading the statestore code: OfferUpdate() takes subscribers_lock_ if the update queue is full, but in one place that lock has already been taken by the caller:

 {
    lock_guard<mutex> l(subscribers_lock_);
    // ... snip ...
    if (state == FailureDetector::FAILED) {
      if (is_heartbeat) {
        // TODO: Consider if a metric to track the number of failures would be useful.
        LOG(INFO) << "Subscriber '" << subscriber->id() << "' has failed, disconnected "
                  << "or re-registered (last known registration ID: " << update.second
                  << ")";
        UnregisterSubscriber(subscriber.get());
      }
    } else {
      // Schedule the next message.
      VLOG(3) << "Next " << (is_heartbeat ? "heartbeat" : "update") << " deadline for: "
              << subscriber->id() << " is in " << deadline_ms << "ms";
      // vvvvvvvvvvvvvvvvvv oops vvvvvvvvvvvvvvvvvv
      OfferUpdate(make_pair(deadline_ms, subscriber->id()), is_heartbeat ?
          &subscriber_heartbeat_threadpool_ : &subscriber_topic_update_threadpool_);
    }
  }

It's not as scary as it sounds, because if the update queue has > 10000 entries there's something wrong anyway, but we should fix this.