Skip to content

Commit

Permalink
Make sure we close all http.Response and io.ReadCloser objs (#1275)
Browse files Browse the repository at this point in the history
* Close several http.Response bodies that were forgotten

Found using https://github.com/timakin/bodyclose

* Close all files read with clients.GetFile

* Close remaining io.ReadCloser I could find
  • Loading branch information
victorges authored May 30, 2024
1 parent a8df5cf commit 528a758
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 5 deletions.
2 changes: 2 additions & 0 deletions balancer/mist/mist_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func (b *MistBalancer) changeLoadBalancerServers(ctx context.Context, server, ac
glog.Errorf("Error making request: %v", err)
return nil, err
}
defer resp.Body.Close()

bytes, err := io.ReadAll(resp.Body)

Expand Down Expand Up @@ -202,6 +203,7 @@ func (b *MistBalancer) getMistLoadBalancerServers(ctx context.Context) (map[stri
glog.Errorf("Error making request: %v", err)
return nil, err
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
b, _ := io.ReadAll(resp.Body)
Expand Down
5 changes: 5 additions & 0 deletions clients/broadcaster_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ func findBroadcaster(c Credentials) (BroadcasterList, error) {
if err != nil {
return BroadcasterList{}, fmt.Errorf("http do(%s): %v", requestURL, err)
}
defer res.Body.Close()

if !httpOk(res.StatusCode) {
return BroadcasterList{}, fmt.Errorf("http GET(%s) returned %d %s", requestURL, res.StatusCode, res.Status)
}
Expand Down Expand Up @@ -124,6 +126,8 @@ func CreateStream(c Credentials, streamName string, profiles []video.EncodedProf
if err != nil {
return "", fmt.Errorf("http do(%s): %v", requestURL, err)
}
defer res.Body.Close()

if !httpOk(res.StatusCode) {
return "", fmt.Errorf("http POST(%s) returned %d %s", requestURL, res.StatusCode, res.Status)
}
Expand Down Expand Up @@ -156,6 +160,7 @@ func ReleaseManifestID(c Credentials, manifestId string) error {
if err != nil {
return fmt.Errorf("Releasing Manifest ID failed. URL: %s, manifestID: %s, err: %s", requestURL, manifestId, err)
}
defer res.Body.Close()

if !httpOk(res.StatusCode) {
return fmt.Errorf("Releasing Manifest ID failed. URL: %s, manifestID: %s, HTTP Code: %s", requestURL, manifestId, res.Status)
Expand Down
6 changes: 5 additions & 1 deletion clients/input_copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ func getSignedURL(osTransferURL *url.URL) (string, error) {
signedURL := httpURL.String()

resp, err := http.Head(signedURL)
if resp != nil {
resp.Body.Close()
}
if err == nil && resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusBadRequest {
return signedURL, nil
}
Expand Down Expand Up @@ -253,7 +256,8 @@ func CopyFileWithDecryption(ctx context.Context, sourceURL, destOSBaseURL, filen
if err != nil {
return fmt.Errorf("error decrypting file: %w", err)
}
c = io.NopCloser(decryptedFile)
defer decryptedFile.Close()
c = decryptedFile
}

content := io.TeeReader(c, &byteAccWriter)
Expand Down
5 changes: 4 additions & 1 deletion clients/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ func DownloadRenditionManifest(requestID, sourceManifestOSURL string) (m3u8.Medi
if err != nil {
return fmt.Errorf("error downloading manifest: %s", err)
}
defer rc.Close()

playlist, playlistType, err = m3u8.DecodeFrom(rc, true)
if err != nil {
return fmt.Errorf("error decoding manifest: %s", err)
Expand Down Expand Up @@ -276,12 +278,13 @@ func ClipInputManifest(requestID, sourceURL, clipTargetUrl string, startTimeUnix
if err != nil {
return fmt.Errorf("error clipping: failed to download segment %d: %w", v.SeqId, err)
}
defer rc.Close()

// Write the segment data to the temp local file
_, err = io.Copy(clipSegmentFile, rc)
if err != nil {
return fmt.Errorf("error clipping: failed to write segment %d: %w", v.SeqId, err)
}
rc.Close()
return nil
}, DownloadRetryBackoff())
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion handlers/geolocation/geolocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,12 @@ func (c *GeolocationHandlersCollection) sendPlaybackRequestAsync(playbackID stri

go func() {
url := fmt.Sprintf("%s/hls/%s+%s/index.m3u8", m.Tags["https"], c.Config.MistBaseStreamName, playbackID)
if _, err := http.Get(url); err != nil {
resp, err := http.Get(url)
if err != nil {
glog.Errorf("Error making a playback request url=%s, err=%v", url, err)
return
}
resp.Body.Close()
}()
}

Expand Down
1 change: 1 addition & 0 deletions mapic/utils/seekinghttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ func (s *SeekingHTTP) Size() (int64, error) {
if err != nil {
return 0, err
}
defer resp.Body.Close()

if resp.ContentLength < 0 {
return 0, errors.New("no content length for Size()")
Expand Down
9 changes: 8 additions & 1 deletion thumbnails/thumbnails.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ func getMediaManifest(requestID string, input string) (*m3u8.MediaPlaylist, erro
if err != nil {
return nil, fmt.Errorf("error downloading manifest: %w", err)
}
defer rc.Close()

manifest, playlistType, err := m3u8.DecodeFrom(rc, true)
if err != nil {
return nil, fmt.Errorf("failed to decode manifest: %w", err)
Expand Down Expand Up @@ -95,7 +97,10 @@ func GenerateThumbsVTT(requestID string, input string, output *url.URL) error {
}
// check thumbnail file exists on storage
err = backoff.Retry(func() error {
_, err := clients.GetFile(context.Background(), requestID, outputLocation.JoinPath(filename).String(), nil)
rc, err := clients.GetFile(context.Background(), requestID, outputLocation.JoinPath(filename).String(), nil)
if rc != nil {
rc.Close()
}
return err
}, thumbWaitBackoff)
if err != nil {
Expand Down Expand Up @@ -199,6 +204,8 @@ func GenerateThumbsFromManifest(requestID, input string, output *url.URL) error
if err != nil {
return fmt.Errorf("error downloading manifest: %w", err)
}
defer rc.Close()

bs, err := io.ReadAll(rc)
if err != nil {
return err
Expand Down
4 changes: 3 additions & 1 deletion transcode/transcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"bytes"
"context"
"fmt"
"github.com/grafov/m3u8"
"math"
"net/url"
"os"
Expand All @@ -14,6 +13,8 @@ import (
"sync"
"time"

"github.com/grafov/m3u8"

"github.com/cenkalti/backoff/v4"
c2pa2 "github.com/livepeer/catalyst-api/c2pa"
"github.com/livepeer/catalyst-api/clients"
Expand Down Expand Up @@ -538,6 +539,7 @@ func transcodeSegment(
if err != nil {
return fmt.Errorf("failed to download source segment %q: %s", segment.Input, err)
}
defer rc.Close()

// If an AccessToken is provided via the request for transcode, then use remote Broadcasters.
// Otherwise, use the local harcoded Broadcaster.
Expand Down

0 comments on commit 528a758

Please sign in to comment.