[Dhriti07]

DO NOT MERGE. Diagnosis only; not a proposal to revert #14668 on main.

Purpose: check whether the Storage Integration hang (ITAppendableUploadTest, ~3h Kokoro timeout since Oct 19) is related to #14668.

Contents:

• retry-budget fix, watchdog thread dumps, and 900s Failsafe fork timeout (same as [Do not Submit] fix(storage): only reset appendable upload retry budget on progress #14710)
• a revert of feat(auth,gax): support certificate-bound tokens and mTLS rotation for agent identities #14668 (01eaba1) for this test branch only

Expected: if Storage ITs finish normally with no WATCHDOG output, #14668 is implicated; if they still hang, it is not.

[gemini-code-assist]

Code Review

This pull request reverts the mTLS certificate rotation and channel refreshing features across several libraries, including the removal of helper utilities like AgentIdentityUtils and RefreshingHttpJsonChannel, and simplifies the channel pool and call context classes. Feedback on these changes highlights several critical resource leak vulnerabilities in ChannelPool due to missing exception handling and unreleased entries during early cancellations. Additionally, unconditionally shutting down the HttpTransport in ManagedHttpJsonChannel poses a risk of prematurely closing shared, user-managed transports.

[gemini-code-assist]

Reverting the try-catch block here reintroduces a potential resource leak. If entry.channel.newCall(methodDescriptor, callOptions) throws a RuntimeException or Error (for example, due to an invalid state or interceptor failure), the acquired entry (via getRetainedEntry) will never be released, leading to a channel entry leak.

      Entry entry = getRetainedEntry(affinity);
      try {
        return new ReleasingClientCall<>(
            entry.channel.newCall(methodDescriptor, callOptions), entry);
      } catch (Throwable t) {
        entry.release();
        throw t;
      }

References

1. Catching Throwable instead of Exception is acceptable at Future boundaries or during stream cleanup in asynchronous operations to ensure all failures are captured and propagated.

[gemini-code-assist]

If the call is cancelled before start() is called, cancellationException will be non-null. Throwing IllegalStateException here without releasing the acquired entry will result in a silent resource leak of the channel entry, as onClose() will never be invoked to release it.

      if (cancellationException != null) {
        if (wasReleased.compareAndSet(false, true)) {
          entry.release();
        }
        throw new IllegalStateException("Call is already cancelled", cancellationException);
      }

[gemini-code-assist]

Catching only Exception here will miss any Throwable that is an Error (such as LinkageError, NoClassDefFoundError, or AssertionError) thrown during super.start(). If an Error is thrown, the entry will not be released, leading to a resource leak. It is safer to catch Throwable to ensure the entry is always released on any failure.

Suggested change

	} catch (Exception e) {
	// In case start failed, make sure to release
	if (wasReleased.compareAndSet(false, true)) {
	entry.release();
	} else {
	LOG.log(
	Level.WARNING,
	"The entry is already released. This indicates that onClose() has already been called previously");
	}
	throw e;
	}
	} catch (Throwable t) {
	// In case start failed, make sure to release
	if (wasReleased.compareAndSet(false, true)) {
	entry.release();
	} else {
	LOG.log(
	Level.WARNING,
	"The entry is already released. This indicates that onClose() has already been called previously");
	}
	throw t;
	}

References

1. Catching Throwable instead of Exception is acceptable at Future boundaries or during stream cleanup in asynchronous operations to ensure all failures are captured and propagated.

[gemini-code-assist]

Unconditionally calling httpTransport.shutdown() here will shut down any custom HttpTransport provided by the user (e.g., via the builder). If the user shares the HttpTransport instance across multiple clients or manages its lifecycle externally, shutting it down here will cause other clients to fail. It is safer to only shut down the transport if it was created by default by the channel itself.