Close response body multiple times after multiple

2019-07-28 06:31发布

问题:

In this post, it is pointed out that response.Body should be closed to avoid resource leak. It is also shown in the overview examples in http package godoc.

In my test code, I send multiple requests to try an API with

resp, err := http.DefaultClient.Do(req)

multiple times in the same function. Is this a bad practice? In this case, do I write defer resp.Body.Close() after each of them, or just once?

url := server.URL + "/ticket/add"                                       
reader = strings.NewReader(`{"id": "test1", "detail": "test1"}`)        
req, err := http.NewRequest("POST", url, reader)                        
assert.Nil(t, err)               

resp, err := http.DefaultClient.Do(req)                                 
assert.Nil(t, err)                                                      

defer resp.Body.Close()                                                 

assert.Equal(t, http.StatusCreated, resp.StatusCode)                    
// add a ticket with same id                                            
reader = strings.NewReader(`{"id": "test1"}`)                           
req, err = http.NewRequest("POST", url, reader)                         
assert.Nil(t, err)                                                      

resp, err = http.DefaultClient.Do(req)                                  
assert.Nil(t, err)                                                      
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)   

A related question, on the server side, i.e., inside the func(w http.ResponseWriter, r *http.Request), is it necessary to close the request body as well?

回答1:

Yes, you need to close both responses. Deferring one call to resp.Body.Close does not somehow effect the other. The *http.Response is different in each case, and they both can be deferred.

On the server side, you do not need to close the Request.Body -- from the http.Request documentation:

// The Server will close the request body. The ServeHTTP
// Handler does not need to.


回答2:

Bad practice

If you did not reuse resp variable, it would be obvious that you are dealing with different response instances each of which must be closed.

Do sends an HTTP request and returns an HTTP response (instance)...

So, the bad practice is not to do several requests but to reuse the same variable for several responses. It causes code unobviousness. And produces unreachable objects which never be finalized.

Deferred execution

A defer statement pushes a function call onto a list. The list of saved calls is executed after the surrounding function returns.

If you have scheduled single or several deferred executions with reference to the same variable only the last assigned to the variable object's method will be executed (several times for several defers).

Playground with an example

Closing Response.Body

From Response documentation:

It is the caller's responsibility to close Body.

So, typically you must close each Response.Body.

Garbage collecting and finalization (edit)

Garbage collector invokes bind to collecting objects finalizers to close files, connections and to do other cleanup actions. And there is no finalizers bind to Body object by default.

Your improved snippet:

// ...a lot of code
resp_one, err := http.DefaultClient.Do(req)                                 
 // BTW, `assert.Nil` just returns whether the assertion was successful (bool) not terminates a test.
if assert.Nil(t, err) == true {
    defer resp_one.Body.Close()                                                 
}

// ...a lot of code
resp_two, err = http.DefaultClient.Do(req)                                  
if assert.Nil(t, err) == true {
    defer resp_two.Body.Close()
}