2

My Setup: I have created a REST service (Jersey/Dropwizard) that streams large content from a database. During a GET operation, the service taps into the database through a connection pool, wraps the data into a stream and performs some on-the-fly transformation to render the requested data in various encodings (CSV, JSON, ...). The life time of the database connection is tied to the life time of the stream and only when the stream is closed, the database connection is released.

The stream transformation is performed by an Encoder class that returns a StreamingOutput which is then passed to the Response object. The Encoder currently handles resource closing when the stream is fully consumed.

My Problem: Since StreamingOutput does not implement AutoCloseable, connection leaks may occur when the output is only partially consumed.

I sometimes observe that stale active connections are piling up in the connection pool, and I suspect that they arise from aborted HTTP connections. As you can see below, the current code handles exceptions that occur in the try block. What I cannot handle are Exceptions that occur after the return statement and I don't know how to attach any instructions for resource closing to the Response object.

My Question: How can I inform the Response object to close particular resources after the request has terminated (regularly or due to an error)? Or: Is there a better way to safely close any associated resources when the request context ends?

 @GET
 //@Produces(...)
 public Response streamData(
        @PathParam("key") String key,
        // ... other params
        ) {

    //decode and validate params
    Stream<Pojo> ps = null;
    try {
        // connect to db and obtain data stream for <key>
        ps = loadData(db, key);
        // apply detailed encoding instrunctions and create a StreamingOutput
        final StreamingOutput stream = Encoder.encodeData(ps, encodingArgs);
        return Response.ok(stream).build();
    } catch (Exception e) {
        closeOnException(ps); // wrapper for ps.close();
        throw e;
    }
 }
  • Just thinking, from my understanding of your code, the real problem seems the reference to StreamingOutput object inside the Encode class (I don't know this Encode class, perhaps you wrote it). An alternative solution, you can extend StreamingOutput and create a class that also contains the loaded data as well, and the Encode is only a builder that does not maintain references to created streams. – Fabiano Tarlao Nov 22 '18 at 17:05
  • In this way, when something goes wrong (everywhere), the GC is able to free the object cause the only reference is in the Response obj, that is going to be freed by GC as well. Is this feasible in your case? In that case I'll convert comment into an answer. – Fabiano Tarlao Nov 22 '18 at 17:06
  • @FabianoTarlao: True, and this is probably the most robust solution. However, that way forces me to construct a StreamingOutput around every block of application logic that streams from the DB and encodes. –  Nov 22 '18 at 18:39
  • @FabianoTarlao: You really want to close I/O resources eagerly and don't wait for the GC. There's simply no guarantee that the GC will clean up the response objects in the near future. –  Nov 22 '18 at 18:45
  • Got it. The other idea is to return an object that implements StreamingOutput interface, perhaps extending your previous class, but that also updates an internal long "last_used_time" variable that can be periodically checked by Encode class. But I don't understand your constraints, can you change/rewrite the internals of StreamingOutput and Encode class, or not? I think that this is my last useful tip, I have not other ideas :-) and all these solutions look too much convoluted ;-) – Fabiano Tarlao Nov 22 '18 at 22:18
  • ... and that is exactly where I am stuck ;-) –  Nov 23 '18 at 07:45
  • Adding "last_used_time" variable by extending does not look very difficult. It is not "beautiful&elegant" but are you facing specific problems? note: I encourage you to not get the system time at each read ;-) – Fabiano Tarlao Nov 23 '18 at 08:45

2 Answers2

2

I received a good answer from the dropwizard mailing list that solves my problem and I want to reference here it in case somebody encounters the same problem.

https://groups.google.com/forum/#!topic/dropwizard-user/62GoLDBrQuo

Citing from Shawn's response:

Jersey supports CloseableService that lets you register Closeable objects to be closed when the request is complete:

public Response streamData(..., @Context CloseableService closer) {
  ...
  closer.add(closeable);
  return Response.ok(...).build();
}
0

You can add to your method HttpServletResponse:

@Produces(MediaType.APPLICATION_OCTET_STREAM)
public Object streamData(
            @PathParam("key") String key,
            @Context HttpServletResponse response,
            // ... other params
            ) {
  ...
  response.getOutputStream().write(....)
  response.flushBuffer();
  response.getOutputStream().close();
  return null;
}
xyz
  • 812
  • 9
  • 18
  • Hmm, I was hoping for some convenient hook (similar to _Stream.onClose(Autocloseable)_) that I could use to avoid boilerplate code. Do you know how Jersey treats the retuned object from the method signature? Is it just discarded? –  Nov 22 '18 at 14:59
  • may be this will be helpfull https://stackoverflow.com/questions/39572872/closing-jax-rs-streamingoutputs-outputstream – xyz Nov 22 '18 at 15:24