ClientUtil does not close Jersey Client objects, possibly resulting in resource leak #13
Description
@dmgaldi discovered that we are creating new Client objects with every request to a number of methods in the fgputil-client package. Relevant Slack discussion below:
Daniel Galdi
I was looking at the makeAsyncRequest API in FGPUtil trying to learn a bit about jersey. I noticed that we create a jakarta.ws.rs.client.Client each time it's called. We might want to share one or a few clients instead of constructing so many:
Clients are heavy-weight objects that manage the client-side communication infrastructure. Initialization as well as disposal of a Client instance may be a rather expensive operation. It is therefore advised to construct only a small number of Client instances in the application. Client instances must be properly closed before being disposed to avoid leaking resources.
Ryan Doherty
Good point Dan, I didn't notice the expense there, though maybe that's a reason not to worry about it? What concerns me more is that we aren't closing these (ever, best I can tell). If you look at the code, there is a bunch of shutdownHook logic in the JerseyClient class, but it's not obvious that it helps us.
I looked around the fgputil client API and I can't see a good way to be backward compatible and also close this up. I was hoping we could send the Client along into a CloseableResponse (further possibly nested inside a ResponseFuture for async requests), but we currently deal in the opaque Future object returned by Jersey's native async method.
So, we should probably pass in the Client, and let the caller figure out how to close it. Starting in Jersey 2.1, it became AutoCloseable, so as a one-off callers can wrap ClientBuilder.newClient() in a try-with-resources. Our EDA services can maybe create one per incoming request that is shared for all its outgoing calls during that request; then we don't have to manage them at any higher level.