Skip to content
This repository was archived by the owner on Jun 19, 2025. It is now read-only.

fix goroutine leak when reloading server config#469

Merged
harshavardhana merged 1 commit into
masterfrom
keystore-leak
Jun 17, 2024
Merged

fix goroutine leak when reloading server config#469
harshavardhana merged 1 commit into
masterfrom
keystore-leak

Conversation

@aead

@aead aead commented Jun 17, 2024

Copy link
Copy Markdown
Member

This commit fixes a goroutine leak that occurs when reloading the server configuration.

During a config reload, the server establishes a 2nd connection to the backend keystore and replaces the existing connection with the newly opened one. The switch is performed atomically (without locking) to not block or abort ongoing requests.

Once the server has replaced the keystore connection, it closes it. Before this commit, the server stopped the in-memory key cache and its GC goroutines. However, it did not close any resources (goroutines/file descriptors) allocated by the replaced keystore. This commit fixes this.

The resource leak can only be observed when reloading the KES server config - i.e. using SIGHUP signals

@aead
aead requested review from donatello and shtripat June 17, 2024 15:21
This commit fixes a goroutine leak that occurs when
reloading the server configuration.

During a config reload, the server establishes a 2nd
connection to the backend keystore and replaces the
existing connection with the newly opened one. The switch
is performed atomically (without locking) to not block
or abort ongoing requests.

Once the server has replaced the keystore connection,
it closes it. Before this commit, the server stopped
the in-memory key cache and its GC goroutines. However,
it did not close any resources (goroutines/file descriptors)
allocated by the replaced keystore. This commit fixes this.

Signed-off-by: Andreas Auernhammer <github@aead.dev>

@klauspost klauspost left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@harshavardhana
harshavardhana merged commit 12195cc into master Jun 17, 2024
@harshavardhana
harshavardhana deleted the keystore-leak branch June 17, 2024 15:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants