open-horizon / edge-sync-service

Cloud - Edge synchronization service (MMS)
Apache License 2.0
24 stars 26 forks source link

Unsuccessful HTTP requests may leak open files #67

Closed dlarson04 closed 3 years ago

dlarson04 commented 3 years ago

See https://github.com/open-horizon/anax/issues/2517 for complete explanation..

There are some code paths in ESS in

https://github.com/open-horizon/edge-sync-service/blob/master/core/communications/httpCommunication.go

that should also be changed to make sure the response body is closed.

These are called less frequently than other code paths in the anax agent but should make the same change here.

dlarson04 commented 3 years ago

@linggao @sirotsinskuy

After crippling my CSS server to return 503 errors, I saw the number of open files owned by anax continue to grow... so I think I am recreating the issue our internal customer saw

By adding the

        if response != nil && response.Body != nil {
                defer response.Body.Close()
        }

to https://github.com/open-horizon/edge-sync-service/blob/cf227ba77d9c835eebc6cc3efcbec32a79eb204c/core/communications/httpCommunication.go#L663

avoided the growing open files in my crippled environment.

Additionally, I would like to add

request.Close = true

to the following methods pushData, ResendObjects, SendNotificationMessage, registerOrPing, unregister, GetData

like we did in the Poll method... We don't need the ESS to maintain a persistent connection to the CSS

dlarson04 commented 3 years ago

Verified with this version Horizon CLI version: 2.29.0-442 Horizon Agent version: 2.29.0-442