No longer spawning goros when statsChan is full, because of uncontrolled memory risk

psiphon/stats_collector.go

@@ -105,24 +105,15 @@ type statsUpdate struct {
 	numBytesReceived int64
 }
 
-// recordStats makes sure the given stats update is added to the global
-// collection. Guaranteed to not block.
+// recordStats adds the given stats update is added to the global collection.
 // Callers of this function should assume that it "takes control" of the
 // statsUpdate object.
 func recordStat(newStat *statsUpdate) {
+	// This function has the potential to block, if statsChan gets full. The
+	// intention is that we give statsChan a big enough buffer that it doesn't
+	// block in normal circumstances
 	statSlice := []*statsUpdate{newStat}
-	// Priority: Don't block connections when updating stats. We can't just
-	// write to the statsChan, since that will block if it's full. We could
-	// launch a goroutine for each update, but that seems like  unnecessary
-	// overhead. So we'll try to write to the channel, and launch a goro if it
-	// fails.
-	select {
-	case allStats.statsChan <- statSlice:
-	default:
-		go func() {
-			allStats.statsChan <- statSlice
-		}()
-	}
+	allStats.statsChan <- statSlice
 }
 
 // processStats is a goro started by Start() and runs until Stop(). It collects

psiphon/stats_test.go

@@ -245,14 +245,11 @@ func (suite *StatsTestSuite) Test_Regex() {
 
 func (suite *StatsTestSuite) Test_recordStat() {
 	// The normal operation of this function will get exercised during the
-	// other tests, but there is a code branch that only gets hit when the
-	// allStats.statsChan is filled. To make sure we fill the channel, we will
-	// lock the stats access mutex, try to record a bunch of stats, and then
-	// release it.
-	allStats.statsMutex.Lock()
+	// other tests. Here we will quickly record more stats updates than the
+	// channel capacity. The test is just that this function returns, and doesn't
+	// crash or block forever.
 	stat := statsUpdate{"test", "test", 1, 1}
 	for i := 0; i < _CHANNEL_CAPACITY*2; i++ {
 		recordStat(&stat)
 	}
-	allStats.statsMutex.Unlock()
 }