Repository: prometheus/prometheus (language: go)

Issue:
panic: close of closed channel
### What did you do?

Running Prometheus and noticed a restart. Checking the logs I see a panic:

```
ts=2024-05-26T11:02:52.087Z caller=main.go:617 level=info msg="Starting Prometheus Server" mode=server version="(version=2.52.0, branch=HEAD, revision=879d80922a227c37df502e7315fad8ceb10a986d)"
ts=2024-05-26T11:02:52.087Z caller=main.go:622 level=info build_context="(go=go1.21.10, platform=linux/arm64, user=root@66505e2b88ab, date=20240513-16:47:19, tags=netgo,builtinassets,stringlabels)"
ts=2024-05-26T11:02:52.087Z caller=main.go:623 level=info host_details="(Linux 6.9.1-1-default #1 SMP PREEMPT_DYNAMIC Fri May 17 11:59:46 UTC 2024 (0c0b0b5) aarch64 prometheus-prometheus-kube-prometheus-prometheus-0 (none))"
ts=2024-05-26T11:02:52.087Z caller=main.go:624 level=info fd_limits="(soft=1048576, hard=1048576)"
ts=2024-05-26T11:02:52.087Z caller=main.go:625 level=info vm_limits="(soft=unlimited, hard=unlimited)"
ts=2024-05-26T11:02:52.091Z caller=web.go:568 level=info component=web msg="Start listening for connections" address=0.0.0.0:9090
ts=2024-05-26T11:02:52.093Z caller=main.go:1129 level=info msg="Starting TSDB ..."
ts=2024-05-26T11:02:52.093Z caller=repair.go:56 level=info component=tsdb msg="Found healthy block" mint=1716699972932 maxt=1716703200000 ulid=01HYSYZ4JRQG8MPRMPVW2MBRM4
ts=2024-05-26T11:02:52.094Z caller=tls_config.go:313 level=info component=web msg="Listening on" address=[::]:9090
ts=2024-05-26T11:02:52.094Z caller=tls_config.go:352 level=info component=web msg="TLS is disabled." http2=false address=[::]:9090
ts=2024-05-26T11:02:52.094Z caller=repair.go:56 level=info component=tsdb msg="Found healthy block" mint=1716703200266 maxt=1716710400000 ulid=01HYT21P4R6Q7XXDGNBDANYS5Q
ts=2024-05-26T11:02:52.095Z caller=repair.go:56 level=info component=tsdb msg="Found healthy block" mint=1716710400266 maxt=1716717600000 ulid=01HYT8XDCR9QQ1D92GQADC6F96
ts=2024-05-26T11:02:52.095Z caller=db.go:960 level=info component=tsdb msg="Found and deleted tmp block dir" dir=/prometheus/wal/checkpoint.00000002.tmp
ts=2024-05-26T11:02:52.095Z caller=dir_locker.go:77 level=warn component=tsdb msg="A lockfile from a previous execution already existed. It was replaced" file=/prometheus/lock
ts=2024-05-26T11:02:52.128Z caller=head.go:616 level=info component=tsdb msg="Replaying on-disk memory mappable chunks if any"
ts=2024-05-26T11:02:52.159Z caller=head.go:703 level=info component=tsdb msg="On-disk memory mappable chunks replay completed" duration=31.087597ms
ts=2024-05-26T11:02:52.159Z caller=head.go:711 level=info component=tsdb msg="Replaying WAL, this may take a while"
ts=2024-05-26T11:02:55.424Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=0 maxSegment=6
ts=2024-05-26T11:02:56.419Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=1 maxSegment=6
ts=2024-05-26T11:02:57.112Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=2 maxSegment=6
ts=2024-05-26T11:03:04.617Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=3 maxSegment=6
ts=2024-05-26T11:03:05.693Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=4 maxSegment=6
ts=2024-05-26T11:03:05.694Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=5 maxSegment=6
ts=2024-05-26T11:03:05.694Z caller=head.go:783 level=info component=tsdb msg="WAL segment loaded" segment=6 maxSegment=6
ts=2024-05-26T11:03:05.694Z caller=head.go:820 level=info component=tsdb msg="WAL replay completed" checkpoint_replay_duration=109.161µs wal_replay_duration=13.535062017s wbl_replay_duration=160ns chunk_snapshot_load_duration=0s mmap_chunk_replay_duration=31.087597ms total_replay_duration=13.566341576s
ts=2024-05-26T11:03:05.849Z caller=main.go:1150 level=info fs_type=9123683e
ts=2024-05-26T11:03:05.849Z caller=main.go:1153 level=info msg="TSDB started"
ts=2024-05-26T11:03:05.849Z caller=main.go:1335 level=info msg="Loading configuration file" filename=/etc/prometheus/config_out/prometheus.env.yaml
ts=2024-05-26T11:03:05.880Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/loki/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T11:03:05.887Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/prometheus-kube-prometheus-apiserver/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T11:03:05.888Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/prometheus-kube-prometheus-kube-scheduler/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T11:03:05.888Z caller=kubernetes.go:331 level=info component="discovery manager notify" discovery=kubernetes config=config-0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T11:03:05.935Z caller=main.go:1372 level=info msg="Completed loading of configuration file" filename=/etc/prometheus/config_out/prometheus.env.yaml totalDuration=85.683225ms db_storage=15.24µs remote_storage=1.96µs web_handler=880ns query_engine=1.04µs scrape=24.480682ms scrape_sd=8.73418ms notify=33.401µs notify_sd=270.403µs rules=46.12161ms tracing=8.68µs
ts=2024-05-26T11:03:05.935Z caller=main.go:1114 level=info msg="Server is ready to receive web requests."
ts=2024-05-26T11:03:05.935Z caller=manager.go:163 level=info component="rule manager" msg="Starting rule manager..."
ts=2024-05-26T12:15:33.829Z caller=main.go:964 level=warn msg="Received an OS signal, exiting gracefully..." signal=terminated
ts=2024-05-26T12:15:39.846Z caller=main.go:988 level=info msg="Stopping scrape discovery manager..."
ts=2024-05-26T12:15:45.444Z caller=main.go:1002 level=info msg="Stopping notify discovery manager..."
ts=2024-05-26T12:15:47.088Z caller=main.go:984 level=info msg="Scrape discovery manager stopped"
ts=2024-05-26T12:15:49.411Z caller=main.go:1335 level=info msg="Loading configuration file" filename=/etc/prometheus/config_out/prometheus.env.yaml
ts=2024-05-26T12:15:53.959Z caller=manager.go:177 level=info component="rule manager" msg="Stopping rule manager..."
ts=2024-05-26T12:15:53.437Z caller=main.go:998 level=info msg="Notify discovery manager stopped"
ts=2024-05-26T12:18:39.836Z caller=manager.go:187 level=info component="rule manager" msg="Rule manager stopped"
ts=2024-05-26T12:18:39.838Z caller=main.go:1039 level=info msg="Stopping scrape manager..."
ts=2024-05-26T12:18:39.852Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/prometheus-kube-prometheus-operator/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T12:18:39.861Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/prometheus-kube-prometheus-kube-proxy/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T12:18:39.933Z caller=kubernetes.go:331 level=info component="discovery manager scrape" discovery=kubernetes config=serviceMonitor/monitoring/prometheus-kube-prometheus-apiserver/0 msg="Using pod service account via in-cluster config"
ts=2024-05-26T12:18:39.941Z caller=kubernetes.go:331 level=info component="discovery manager notify" discovery=kubernetes config=config-0 msg="Using pod service account via in-cluster config"
panic: close of closed channel

goroutine 7768 [running]:
github.com/prometheus/prometheus/rules.(*Group).stop(...)
        /bitnami/blacksmith-sandox/prometheus-2.52.0/src/github.com/prometheus/prometheus/rules/group.go:260
github.com/prometheus/prometheus/rules.(*Manager).Update.func2({0x4014cd3080, 0xa2}, 0x400753cfc0)
        /bitnami/blacksmith-sandox/prometheus-2.52.0/src/github.com/prometheus/prometheus/rules/manager.go:241 +0x44
created by github.com/prometheus/prometheus/rules.(*Manager).Update in goroutine 245
        /bitnami/blacksmith-sandox/prometheus-2.52.0/src/github.com/prometheus/prometheus/rules/manager.go:239 +0x344
```

### What did you expect to see?

No panics

### What did you see instead? Under which circumstances?

Panics

### System information

_No response_

### Prometheus version

```text
2.52.0
879d80922a227c37df502e7315fad8ceb10a986d
HEAD
root@66505e2b88ab
20240513-16:47:19
go1.21.10
```


### Prometheus configuration file

_No response_

### Alertmanager version

_No response_

### Alertmanager configuration file

_No response_

### Logs

_No response_


Discussion / hints:
Thank you for providing the logs! 

This looks like a bug and not it Is not clear if it is a regression. I will try to reproduce and provide a fix.
I just confirmed my hunch:

This panic can happen if the configuration for the rule groups is reloaded while prometheus is being shutdown:
* t1: Ruler is shutdown [here](https://github.com/prometheus/prometheus/blob/main/cmd/prometheus/main.go#L1033) after the process receive the TERM signal [here](https://github.com/prometheus/prometheus/blob/main/cmd/prometheus/main.go#L981). During shutdown, the groups are [closed](https://github.com/prometheus/prometheus/blob/main/rules/manager.go#L181).
* t2: process receive the HUP signal [here](https://github.com/prometheus/prometheus/blob/main/cmd/prometheus/main.go#L1089)
 and configuration is reloaded. Please notice that this actor hasn't received the cancel from the channel yet because the shutdown of actors is processed sequentially. During the configuration reload, the groups are [closed again](https://github.com/prometheus/prometheus/blob/main/rules/manager.go#L242), causing the panic.

One approach to solve this problem that I'm thinking of is to make the `Reload` method in the manager noop in case the manager has already stopped. It does not make sense to reload a closed manager. 

Another alternative is to make the `Reload` function error in case it is already closed, but also include a flag that allow consumers of the manager to know its state so that they don't call `Reload`.

I'm inclined to implement the first approach.



Fix patch:
diff --git a/rules/manager.go b/rules/manager.go
index 063189e0ab1..acc637e7187 100644
--- a/rules/manager.go
+++ b/rules/manager.go
@@ -190,10 +190,18 @@ func (m *Manager) Stop() {
 
 // Update the rule manager's state as the config requires. If
 // loading the new rules failed the old rule set is restored.
+// This method will no-op in case the manager is already stopped.
 func (m *Manager) Update(interval time.Duration, files []string, externalLabels labels.Labels, externalURL string, groupEvalIterationFunc GroupEvalIterationFunc) error {
 	m.mtx.Lock()
 	defer m.mtx.Unlock()
 
+	// We cannot update a stopped manager
+	select {
+	case <-m.done:
+		return nil
+	default:
+	}
+
 	groups, errs := m.LoadGroups(interval, externalLabels, externalURL, groupEvalIterationFunc, files...)
 
 	if errs != nil {


Test patch:
diff --git a/rules/manager_test.go b/rules/manager_test.go
index 3bf5fac32be..51239e6c904 100644
--- a/rules/manager_test.go
+++ b/rules/manager_test.go
@@ -2099,6 +2099,23 @@ func TestBoundedRuleEvalConcurrency(t *testing.T) {
 	require.EqualValues(t, maxInflight.Load(), int32(maxConcurrency)+int32(groupCount))
 }
 
+func TestUpdateWhenStopped(t *testing.T) {
+	files := []string{"fixtures/rules.yaml"}
+	ruleManager := NewManager(&ManagerOptions{
+		Context: context.Background(),
+		Logger:  log.NewNopLogger(),
+	})
+	ruleManager.start()
+	err := ruleManager.Update(10*time.Second, files, labels.EmptyLabels(), "", nil)
+	require.NoError(t, err)
+	require.NotEmpty(t, ruleManager.groups)
+
+	ruleManager.Stop()
+	// Updates following a stop are no-op.
+	err = ruleManager.Update(10*time.Second, []string{}, labels.EmptyLabels(), "", nil)
+	require.NoError(t, err)
+}
+
 const artificialDelay = 250 * time.Millisecond
 
 func optsFactory(storage storage.Storage, maxInflight, inflightQueries *atomic.Int32, maxConcurrent int64) *ManagerOptions {