Fix leaky socket #8
Conversation
There was a problem hiding this comment.
@murphybytes I read through the PR and think we should make the http.Client a field on Settings.
This way, the user can provide a client they're already using and you can create a client if the one is settings is nil.
tuf/tuf.go
Outdated
| @@ -46,31 +46,56 @@ type Settings struct { | |||
| TargetName targetNameType | |||
| } | |||
|
|
|||
| type Monitor interface { | |||
There was a problem hiding this comment.
Needs documentation, but I think this interface is a bit too much. Just export the struct.
tuf/tuf.go
Outdated
| client: &http.Client{ | ||
| // New instantiatest and monitor which is used to detect and manage changes to | ||
| // the TUF repository. | ||
| func New(settings *Settings) *Monitor { |
There was a problem hiding this comment.
This is better, but is there still a reason to have the monitor struct if it only holds the Settings struct?
stagingPath, err := tuf.GetStagedPath(&u.settings)
This is the previous API and it can still work now as a function instead of a method.
There was a problem hiding this comment.
I agree this should be changed, but perhaps for another reason than you think. I'm passing a pointer to the Settings object to the loop goroutine so we could have concurrent access problems.
There was a problem hiding this comment.
The client should be safe for concurrent use.
This PR changes the usage http.Client of the updater so that all web requests use the same connection. Also a change was made that will make sure that backed up tuf repo files will be deleted after we are done with them.