Skip to content

Commit ebe969b

Browse files
ibrahimkk-moideenibrahimkk-moideen
authored
Merge pull request #115 from criteo-forks/fix-leaks
Fix various leaks
2 parents 9f35532 + b5f64fb commit ebe969b

File tree

5 files changed

+34
-26
lines changed

5 files changed

+34
-26
lines changed

‎common/util.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func Fetch(uri, host, profile string, client *retryablehttp.Client) func() ([]by
5050
if err != nil {
5151
return nil, err
5252
}
53-
defer resp.Body.Close()
53+
defer EmptyAndCloseBody(resp)
5454
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
5555
if resp.StatusCode == http.StatusNotFound {
5656
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
@@ -59,6 +59,7 @@ func Fetch(uri, host, profile string, client *retryablehttp.Client) func() ([]by
5959
if err != nil {
6060
return nil, err
6161
}
62+
defer EmptyAndCloseBody(resp)
6263
retryCount = retryCount + 1
6364
}
6465
if err != nil {
@@ -86,6 +87,7 @@ func Fetch(uri, host, profile string, client *retryablehttp.Client) func() ([]by
8687
if err != nil {
8788
return nil, fmt.Errorf("Retry DoRequest failed - " + err.Error())
8889
}
90+
defer EmptyAndCloseBody(resp)
8991
if resp.StatusCode == http.StatusUnauthorized {
9092
return nil, ErrInvalidCredential
9193
}
@@ -102,6 +104,15 @@ func Fetch(uri, host, profile string, client *retryablehttp.Client) func() ([]by
102104
}
103105
}
104106

107+
// This is required to have a proper cleanup of the response body
108+
// to have correctly working keep-alive connections
109+
func EmptyAndCloseBody(resp *http.Response) {
110+
if resp.Body != nil {
111+
io.Copy(io.Discard, resp.Body)
112+
resp.Body.Close()
113+
}
114+
}
115+
105116
func BuildRequest(uri, host string) *retryablehttp.Request {
106117
var user, password string
107118

‎exporter/exporter.go

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,13 @@ func NewExporter(ctx context.Context, target, uri, profile, model string, exclud
142142
ExpectContinueTimeout: 1 * time.Second,
143143
TLSClientConfig: &tls.Config{
144144
InsecureSkipVerify: config.GetConfig().SSLVerify,
145+
Renegotiation: tls.RenegotiateOnceAsClient,
145146
},
146147
TLSHandshakeTimeout: 10 * time.Second,
147148
}
148149

150+
defer tr.CloseIdleConnections()
151+
149152
retryClient := retryablehttp.NewClient()
150153
retryClient.CheckRetry = retryablehttp.ErrorPropagatedRetryPolicy
151154
retryClient.HTTPClient.Transport = tr
@@ -526,11 +529,7 @@ func (e *Exporter) collectMetrics(metrics chan<- prometheus.Metric) {
526529
}
527530

528531
func (e *Exporter) scrape() {
529-
530-
var result uint8
531532
state := uint8(1)
532-
scrapes := len(e.pool.Tasks)
533-
scrapeChan := make(chan uint8, scrapes)
534533

535534
// Concurrently call the endpoints to help prevent reaching the maxiumum number of 4 simultaneous sessions
536535
e.pool.Run()
@@ -562,22 +561,14 @@ func (e *Exporter) scrape() {
562561
}
563562

564563
if err != nil {
564+
state = 0
565565
log.Error("error exporting metrics", zap.Error(err), zap.String("url", task.URL), zap.Any("trace_id", e.ctx.Value("traceID")))
566566
continue
567567
}
568-
scrapeChan <- 1
569-
}
570-
571-
// Get scrape results from goroutine(s) and perform bitwise AND, any failures should
572-
// result in a scrape failure
573-
for i := 0; i < scrapes; i++ {
574-
result = <-scrapeChan
575-
state &= result
576568
}
577569

578570
var upMetric = (*e.DeviceMetrics)["up"]
579571
(*upMetric)["up"].WithLabelValues().Set(float64(state))
580-
581572
}
582573

583574
func (e *Exporter) GetContext() context.Context {

‎exporter/helpers.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func getMemberUrls(url, host string, client *retryablehttp.Client) ([]string, er
4040
if err != nil {
4141
return urls, err
4242
}
43-
defer resp.Body.Close()
43+
defer common.EmptyAndCloseBody(resp)
4444
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
4545
if resp.StatusCode == http.StatusUnauthorized {
4646
return urls, common.ErrInvalidCredential
@@ -77,7 +77,7 @@ func getSystemEndpoints(chassisUrls []string, host string, client *retryablehttp
7777
if err != nil {
7878
return sysEnd, err
7979
}
80-
defer resp.Body.Close()
80+
defer common.EmptyAndCloseBody(resp)
8181
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
8282
if resp.StatusCode == http.StatusUnauthorized {
8383
return sysEnd, common.ErrInvalidCredential
@@ -218,7 +218,7 @@ func getSystemsMetadata(url, host string, client *retryablehttp.Client) (oem.Sys
218218
if err != nil {
219219
return sys, err
220220
}
221-
defer resp.Body.Close()
221+
defer common.EmptyAndCloseBody(resp)
222222
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
223223
return sys, fmt.Errorf("HTTP status %d", resp.StatusCode)
224224
}
@@ -247,15 +247,16 @@ func getDIMMEndpoints(url, host string, client *retryablehttp.Client) (oem.Colle
247247
if err != nil {
248248
return dimms, err
249249
}
250-
defer resp.Body.Close()
250+
defer common.EmptyAndCloseBody(resp)
251251
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
252252
if resp.StatusCode == http.StatusNotFound {
253-
for retryCount < 1 && resp.StatusCode == http.StatusNotFound {
253+
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
254254
time.Sleep(client.RetryWaitMin)
255255
resp, err = common.DoRequest(client, req)
256256
if err != nil {
257257
return dimms, err
258258
}
259+
defer common.EmptyAndCloseBody(resp)
259260
retryCount = retryCount + 1
260261
}
261262
if err != nil {
@@ -294,7 +295,7 @@ func getDriveEndpoint(url, host string, client *retryablehttp.Client) (oem.Gener
294295
if err != nil {
295296
return drive, err
296297
}
297-
defer resp.Body.Close()
298+
defer common.EmptyAndCloseBody(resp)
298299
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
299300
if resp.StatusCode == http.StatusNotFound {
300301
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
@@ -303,6 +304,7 @@ func getDriveEndpoint(url, host string, client *retryablehttp.Client) (oem.Gener
303304
if err != nil {
304305
return drive, err
305306
}
307+
defer common.EmptyAndCloseBody(resp)
306308
retryCount = retryCount + 1
307309
}
308310
if err != nil {
@@ -473,7 +475,7 @@ func getProcessorEndpoints(url, host string, client *retryablehttp.Client) (oem.
473475
if err != nil {
474476
return processors, err
475477
}
476-
defer resp.Body.Close()
478+
defer common.EmptyAndCloseBody(resp)
477479
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
478480
if resp.StatusCode == http.StatusNotFound {
479481
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
@@ -482,6 +484,7 @@ func getProcessorEndpoints(url, host string, client *retryablehttp.Client) (oem.
482484
if err != nil {
483485
return processors, err
484486
}
487+
defer common.EmptyAndCloseBody(resp)
485488
retryCount = retryCount + 1
486489
}
487490
if err != nil {

‎exporter/moonshot/exporter.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ func fetch(uri, device, metricType, host, profile string, client *retryablehttp.
206206
if err != nil {
207207
return nil, device, metricType, err
208208
}
209-
defer resp.Body.Close()
209+
defer common.EmptyAndCloseBody(resp)
210210
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
211211
if resp.StatusCode == http.StatusNotFound {
212212
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
@@ -215,6 +215,7 @@ func fetch(uri, device, metricType, host, profile string, client *retryablehttp.
215215
if err != nil {
216216
return nil, device, metricType, err
217217
}
218+
defer common.EmptyAndCloseBody(resp)
218219
retryCount = retryCount + 1
219220
}
220221
if err != nil {
@@ -239,6 +240,7 @@ func fetch(uri, device, metricType, host, profile string, client *retryablehttp.
239240

240241
time.Sleep(client.RetryWaitMin)
241242
resp, err = common.DoRequest(client, req)
243+
defer common.EmptyAndCloseBody(resp)
242244
if err != nil {
243245
return nil, device, metricType, fmt.Errorf("HTTP status %d", resp.StatusCode)
244246
}

‎plugins/nuova/helpers.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,13 @@ func checkRaidController(url, host string, client *retryablehttp.Client) (bool,
8282
defer resp.Body.Close()
8383
if !(resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices) {
8484
if resp.StatusCode == http.StatusNotFound {
85-
for retryCount < 1 && resp.StatusCode == http.StatusNotFound {
85+
for retryCount < 3 && resp.StatusCode == http.StatusNotFound {
8686
time.Sleep(client.RetryWaitMin)
8787
resp, err = common.DoRequest(client, req)
8888
if err != nil {
8989
return false, nil
9090
}
91+
defer common.EmptyAndCloseBody(resp)
9192
retryCount = retryCount + 1
9293
}
9394
if err != nil {
@@ -133,7 +134,7 @@ func IMCPost(uri, classId, cookie string, client *retryablehttp.Client) ([]byte,
133134
if err != nil {
134135
return nil, err
135136
}
136-
defer resp.Body.Close()
137+
defer common.EmptyAndCloseBody(resp)
137138

138139
body, err := io.ReadAll(resp.Body)
139140
if err != nil {
@@ -151,7 +152,7 @@ func IMCLogin(uri, target string, client *retryablehttp.Client) (string, error)
151152
if err != nil {
152153
return aaaLogin.OutCookie, err
153154
}
154-
defer resp.Body.Close()
155+
defer common.EmptyAndCloseBody(resp)
155156

156157
body, err := io.ReadAll(resp.Body)
157158
if err != nil {
@@ -177,7 +178,7 @@ func IMCLogout(uri, cookie string, client *retryablehttp.Client) error {
177178
if err != nil {
178179
return err
179180
}
180-
defer resp.Body.Close()
181+
defer common.EmptyAndCloseBody(resp)
181182

182183
return nil
183184
}

0 commit comments

Comments
 (0)