Make Rest.Response Closeable

[alexey-grigorovich-clearscale]

The following client code triggers a resource leak when handling a non-2xx response and requires a try-finally to work correctly.

val response = api.post("http://example.com/api/entities", entity)
if (response.ok()) {
  doSomething(response.one())
} else {
  throw RuntimeError("request failed")
}

Having Response inherit from Closeable would allow Closeable.use extension function to correctly close the response object.

Alternatively, post() could accept a lambda receiving a response object, but that place is already taken by a request processor.

[edvin]

I like your idea of implementing Closeable. I'll investigate this later today and get back to you.

[edvin]

Rest.Response now implements Closeable. But in your example, there was no resource leak FAIK. The reason is that post does a preemtive fetch of the data, and hence calls consume() automatically. I've added a test case for the closeable feature like this:

@Test
fun testAutoclose() {
    val hangingResponse = api.get("/category")
    FX.runAndWait {
        Assert.assertEquals(1, Rest.ongoingRequests.size)
    }
    hangingResponse.use {
    }
    FX.runAndWait {
        Assert.assertEquals(0, Rest.ongoingRequests.size)
    }
}

The reson the asserts are run inside FX.runAndWait is that the request is removed from the queue on the FX Application Thread. Does this look good to you?

[edvin]

TornadoFX 1.6.1 is released with this improvement :)

[alexey-grigorovich-clearscale]

Looks good, thanks!

Not sure what you mean by post calling consume automatically: I was definitely seeing httpclient's thread pool exhausted when not issuing explicit calls to consume(). I'm on 1.5.9, though.

[edvin]

Great! That might only have been true for the HttpURLConnection version, I didn't check the HttpClient version :)