[API CHANGE] session.go: ensure we close idle connections

cmd/miniooni/main.go

@@ -209,6 +209,7 @@ func main() {
 	if err != nil {
 		log.WithError(err).Fatal("cannot create measurement session")
 	}
+	defer sess.Close()
 
 	if globalOptions.bouncerURL != "" {
 		sess.AddAvailableHTTPSBouncer(globalOptions.bouncerURL)

experiment/mktesting/mktesting.go

@@ -31,6 +31,7 @@ func Run(input string, factory func() model.ExperimentMeasurer) error {
 	if err != nil {
 		return err
 	}
+	defer sess.Close()
 	if err := sess.MaybeLookupBackends(); err != nil {
 		return err
 	}

experiment_test.go

@@ -19,6 +19,7 @@ import (
 
 func TestCreateAll(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	for _, name := range AllExperiments() {
 		builder, err := sess.NewExperimentBuilder(name)
 		if err != nil {
@@ -33,6 +34,7 @@ func TestCreateAll(t *testing.T) {
 
 func TestRunDASH(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("dash")
 	if err != nil {
 		t.Fatal(err)
@@ -42,6 +44,7 @@ func TestRunDASH(t *testing.T) {
 
 func TestRunExample(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -51,6 +54,7 @@ func TestRunExample(t *testing.T) {
 
 func TestRunPsiphon(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("psiphon")
 	if err != nil {
 		t.Fatal(err)
@@ -60,6 +64,7 @@ func TestRunPsiphon(t *testing.T) {
 
 func TestRunSNIBlocking(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("sni_blocking")
 	if err != nil {
 		t.Fatal(err)
@@ -69,6 +74,7 @@ func TestRunSNIBlocking(t *testing.T) {
 
 func TestRunTelegram(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("telegram")
 	if err != nil {
 		t.Fatal(err)
@@ -78,6 +84,7 @@ func TestRunTelegram(t *testing.T) {
 
 func TestRunTor(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("tor")
 	if err != nil {
 		t.Fatal(err)
@@ -87,6 +94,7 @@ func TestRunTor(t *testing.T) {
 
 func TestNeedsInput(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("web_connectivity")
 	if err != nil {
 		t.Fatal(err)
@@ -98,6 +106,7 @@ func TestNeedsInput(t *testing.T) {
 
 func TestSetCallbacks(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -133,6 +142,7 @@ func (c *registerCallbacksCalled) OnProgress(percentage float64, message string)
 
 func TestCreateInvalidExperiment(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("antani")
 	if err == nil {
 		t.Fatal("expected an error here")
@@ -144,6 +154,7 @@ func TestCreateInvalidExperiment(t *testing.T) {
 
 func TestMeasurementFailure(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -165,6 +176,7 @@ func TestMeasurementFailure(t *testing.T) {
 
 func TestUseOptions(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -246,6 +258,7 @@ func TestRunHHFM(t *testing.T) {
 		t.Skip("Measurement Kit not available; skipping")
 	}
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("http_header_field_manipulation")
 	if err != nil {
 		t.Fatal(err)
@@ -388,6 +401,7 @@ func TestSetOption(t *testing.T) {
 
 func TestLoadMeasurement(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -440,6 +454,7 @@ func TestLoadMeasurement(t *testing.T) {
 
 func TestSaveMeasurementErrors(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -485,6 +500,7 @@ func TestSaveMeasurementErrors(t *testing.T) {
 
 func TestOpenReportIdempotent(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -522,6 +538,7 @@ func TestOpenReportFailure(t *testing.T) {
 	))
 	defer server.Close()
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -541,6 +558,7 @@ func TestOpenReportFailure(t *testing.T) {
 
 func TestSubmitAndUpdateMeasurementWithClosedReport(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	builder, err := sess.NewExperimentBuilder("example")
 	if err != nil {
 		t.Fatal(err)
@@ -555,6 +573,7 @@ func TestSubmitAndUpdateMeasurementWithClosedReport(t *testing.T) {
 
 func TestMeasureLookupLocationFailure(t *testing.T) {
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	exp := NewExperiment(sess, new(antaniMeasurer))
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // so we fail immediately
@@ -565,6 +584,7 @@ func TestMeasureLookupLocationFailure(t *testing.T) {
 
 func TestOpenReportNonHTTPS(t *testing.T) {
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	sess.availableCollectors = []model.Service{
 		model.Service{
 			Address: "antani",

oonimkall/runner.go

@@ -124,6 +124,7 @@ func (r *runner) Run(ctx context.Context) {
 		r.emitter.EmitFailureStartup(err.Error())
 		return
 	}
+	defer sess.Close()
 
 	// TODO(bassosimone): set experiment options here
 	// TODO(bassosimone): we should probably also set callbacks here?

session.go

@@ -151,6 +151,15 @@ func (s *Session) CABundlePath() string {
 	return filepath.Join(s.assetsDir, resources.CABundleName)
 }
 
+// Close ensures that we close all the idle connections that the HTTP clients
+// we are currently using may have created. Not calling this function may likely
+// cause memory leaks in your application because of open idle connections.
+func (s *Session) Close() error {
+	s.httpDefaultClient.CloseIdleConnections()
+	s.httpNoProxyClient.CloseIdleConnections()
+	return nil
+}
+
 // CountryDatabasePath is like ASNDatabasePath but for the country DB path.
 func (s *Session) CountryDatabasePath() string {
 	return filepath.Join(s.assetsDir, resources.CountryDatabaseName)

session_test.go

@@ -114,6 +114,7 @@ func newSessionForTesting(t *testing.T) *Session {
 
 func TestIntegrationNewOrchestraClient(t *testing.T) {
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	clnt, err := sess.NewOrchestraClient(context.Background())
 	if err != nil {
 		t.Fatal(err)
@@ -127,6 +128,7 @@ func TestUnitInitOrchestraClientMaybeRegisterError(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // so we fail immediately
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	clnt := orchestra.NewClient(
 		sess.DefaultHTTPClient(),
 		sess.Logger(),
@@ -147,6 +149,7 @@ func TestUnitInitOrchestraClientMaybeRegisterError(t *testing.T) {
 func TestUnitInitOrchestraClientMaybeLoginError(t *testing.T) {
 	ctx := context.Background()
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	clnt := orchestra.NewClient(
 		sess.DefaultHTTPClient(),
 		sess.Logger(),
@@ -181,6 +184,7 @@ func TestBouncerError(t *testing.T) {
 		t.Fatal(err)
 	}
 	sess := newSessionForTestingNoLookupsWithProxyURL(t, URL)
+	defer sess.Close()
 	if sess.ExplicitProxy() == false {
 		t.Fatal("expected to see explicit proxy here")
 	}
@@ -191,6 +195,7 @@ func TestBouncerError(t *testing.T) {
 
 func TestIntegrationSessionLocationLookup(t *testing.T) {
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	if err := sess.MaybeLookupLocation(); err != nil {
 		t.Fatal(err)
 	}
@@ -230,6 +235,7 @@ func TestIntegrationSessionDownloadResources(t *testing.T) {
 	}
 	ctx := context.Background()
 	sess := newSessionForTestingNoLookups(t)
+	defer sess.Close()
 	sess.SetAssetsDir(tmpdir)
 	err = sess.FetchResourcesIdempotent(ctx)
 	if err != nil {
@@ -261,6 +267,7 @@ func TestUnitGetAvailableBouncers(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer sess.Close()
 	all := sess.GetAvailableBouncers()
 	if len(all) != 1 {
 		t.Fatal("unexpected number of bouncers")
@@ -284,6 +291,7 @@ func TestUnitMaybeLookupBackendsFailure(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer sess.Close()
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // so we fail immediately
 	err = sess.MaybeLookupBackendsContext(ctx)
@@ -303,6 +311,7 @@ func TestIntegrationMaybeLookupTestHelpersIdempotent(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer sess.Close()
 	ctx := context.Background()
 	if err = sess.MaybeLookupTestHelpersContext(ctx); err != nil {
 		t.Fatal(err)
@@ -326,6 +335,7 @@ func TestUnitAllBouncersUnsupported(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer sess.Close()
 	sess.AppendAvailableBouncer(model.Service{
 		Address: "mascetti",
 		Type:    "antani",

testlists_test.go

@@ -6,6 +6,7 @@ import (
 
 func TestIntegrationQueryTestListsURLs(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	config := &TestListsURLsConfig{}
 	config.AddCategory("NEWS")
 	config.Limit = 7
@@ -41,6 +42,7 @@ func TestIntegrationQueryTestListsURLs(t *testing.T) {
 
 func TestUnitQueryTestListsURLsQueryFailure(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	config := &TestListsURLsConfig{BaseURL: "\t"}
 	result, err := sess.QueryTestListsURLs(config)
 	if err == nil {
@@ -53,6 +55,7 @@ func TestUnitQueryTestListsURLsQueryFailure(t *testing.T) {
 
 func TestUnitQueryTestListsURLsNilConfig(t *testing.T) {
 	sess := newSessionForTesting(t)
+	defer sess.Close()
 	result, err := sess.QueryTestListsURLs(nil)
 	if err == nil {
 		t.Fatal("expected an error here")