fix: Set TCCL before initialising A2AHttpClientFactory#873
Conversation
It has a ServiceLoader using the TCCL to find clients
There was a problem hiding this comment.
Code Review
This pull request introduces a Thread Context Class Loader (TCCL) swap within the BasePushNotificationSender constructor to ensure that A2AHttpClientFactory is initialized correctly. However, the review feedback points out that this implementation is fragile because it relies on BasePushNotificationSender being the first class to trigger the factory's static initialization. It is recommended to refactor the service discovery logic within A2AHttpClientFactory itself to use a specific class loader, ensuring providers are consistently found regardless of the calling context.
| ClassLoader originalTccl = Thread.currentThread().getContextClassLoader(); | ||
| try { | ||
| Thread.currentThread().setContextClassLoader(BasePushNotificationSender.class.getClassLoader()); | ||
| this.httpClient = A2AHttpClientFactory.create(); | ||
| } finally { | ||
| Thread.currentThread().setContextClassLoader(originalTccl); | ||
| } |
There was a problem hiding this comment.
This TCCL (Thread Context Class Loader) swap is fragile because it depends on the initialization order of A2AHttpClientFactory. Since the factory's PROVIDERS list is initialized in a static block, this logic only works if BasePushNotificationSender is the first class to trigger the factory's loading. If another component accesses the factory first without this TCCL swap, the factory will be initialized with an empty provider list and remain broken for the life of the application. The service discovery should be made robust within A2AHttpClientFactory itself, for example by using ServiceLoader.load(A2AHttpClientProvider.class, A2AHttpClientProvider.class.getClassLoader()) to ensure the providers are always found regardless of the current thread's context.
It has a ServiceLoader using the TCCL to find clients