Issue 4797:Prevent thread leak when BookKeeper client constructor fails

[kroosxu]

Descriptions of the changes in this PR:

Fix #4797

Motivation

When BookKeeper.forConfig(conf).build() fails part-way through construction
(for example because metadataServiceUri points to an unreachable or invalid
ZooKeeper ensemble, or MetadataDrivers#getClientDriver(URI) cannot resolve
a driver for the configured scheme), the constructor leaks every resource it
has already allocated up to the point of failure.

Changes

• In the BookKeeper constructor, when initialization fails after the
  worker pools have been created but before the object is
  fully constructed, invoke close() on the partially-initialized instance
  so that all already-allocated resources are
  shut down before the exception is propagated to the caller.

• Add BookKeeperConstructorFailureTest under
  bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ covering
  the regression:

  • testRepeatedFailedBuildsDoNotAccumulateThreads — 5 consecutive
    failing build() calls must not accumulate threads.

[kroosxu]

Hi @lhotari @merlimat @hangc0276 @StevenLuMT , can you please take a look?

[Copilot]

Pull request overview

This PR addresses a resource/thread leak in BookKeeper client construction by ensuring that resources allocated before a constructor failure are released, and adds a regression test to verify that repeated failed builds do not accumulate client worker threads.

Changes:

• Invoke close() when constructor initialization fails during metadata driver initialization or ledger manager factory initialization.
• Make close() tolerant of partially-initialized instances by null-guarding key fields before dereference.
• Add BookKeeperConstructorFailureTest to detect thread accumulation across repeated failing build() attempts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java	Adds constructor cleanup on specific failure paths and hardens close() against partially-initialized state.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperConstructorFailureTest.java	Adds regression coverage to ensure repeated failed builds don’t accumulate client executor threads.

Comments suppressed due to low confidence (1)

bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java:468

• close() can throw InterruptedException (and BKException per signature), which would override the original MetadataException and change the exception observed by callers. Consider making the cleanup best-effort here (suppress close() failures and preserve interrupt status) so the original initialization error is what’s thrown.

        } catch (MetadataException me) {
            log.error().exception(me).log("Encountered metadata exceptions on initializing metadata client driver");
            close();
            throw new IOException("Failed to initialize metadata client driver", me);
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.