Fix panic when Hearbeat monitor initialization fails twice (#25073)

jsoriano · jsoriano · commit b053f1852876 · 2021-04-14T17:45:55.000+02:00
Initialization was only tried once. Second time a nil object and a nil error are returned, producing nil pointer dereferences later. (cherry picked from commit 1d858bd)
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -214,6 +214,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fixed excessive memory usage introduced in 7.5 due to over-allocating memory for HTTP checks. {pull}15639[15639]
 - Fixed scheduler shutdown issues which would in rare situations cause a panic due to semaphore misuse. {pull}16397[16397]
 - Fixed TCP TLS checks to properly validate hostnames, this broke in 7.x and only worked for IP SANs. {pull}17549[17549]
+- Fix panic when initialization of ICMP monitors fail twice. {pull}25073[25073]
 
 *Journalbeat*
 
diff --git a/heartbeat/monitors/active/icmp/stdloop.go b/heartbeat/monitors/active/icmp/stdloop.go
@@ -87,20 +87,25 @@ type requestResult struct {
 // These vars should not be used directly, but rather getStdLoop
 // should be invoked to initialize and return stdLoop.
 var (
-	stdICMPLoopInit      sync.Once
+	stdICMPLoopInit      sync.Mutex
 	stdICMPLoopSingleton *stdICMPLoop
 )
 
 func getStdLoop() (*stdICMPLoop, error) {
-	var loopErr error
-	stdICMPLoopInit.Do(func() {
+	stdICMPLoopInit.Lock()
+	defer stdICMPLoopInit.Unlock()
+
+	if stdICMPLoopSingleton == nil {
 		debugf("initializing ICMP loop")
-		stdICMPLoopSingleton, loopErr = newICMPLoop()
-		if loopErr == nil {
-			debugf("ICMP loop successfully initialized")
+		singleton, err := newICMPLoop()
+		if err != nil {
+			return nil, err
 		}
-	})
-	return stdICMPLoopSingleton, loopErr
+		stdICMPLoopSingleton = singleton
+		debugf("ICMP loop successfully initialized")
+	}
+
+	return stdICMPLoopSingleton, nil
 }
 
 func noPingCapabilityError(message string) error {
