From 51787ee1bf18a490eec5c63a7c549bfe2f703867 Mon Sep 17 00:00:00 2001 From: Wellington Mafra Date: Wed, 22 Jul 2026 17:30:11 -0300 Subject: [PATCH] fix(go): stop replacing host process signal handlers during install --- .../marcnuri/helm/HelmSignalHandlerTest.java | 101 ++++++++++++++++++ native/internal/helm/install.go | 23 ++-- 2 files changed, 107 insertions(+), 17 deletions(-) create mode 100644 helm-java/src/test/java/com/marcnuri/helm/HelmSignalHandlerTest.java diff --git a/helm-java/src/test/java/com/marcnuri/helm/HelmSignalHandlerTest.java b/helm-java/src/test/java/com/marcnuri/helm/HelmSignalHandlerTest.java new file mode 100644 index 00000000..989c87c9 --- /dev/null +++ b/helm-java/src/test/java/com/marcnuri/helm/HelmSignalHandlerTest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2024 Marc Nuri + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.marcnuri.helm; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that the host JVM keeps its OS-level termination signal handlers after native helm + * calls that used to register Go signal handlers (install path). + * + *

The Go runtime of the c-shared native library permanently replaced the JVM's SIGINT/SIGTERM + * handlers on the first {@code signal.Notify}, leaving the JVM unable to react to SIGTERM + * (shutdown hooks never ran, Kubernetes pods hung until SIGKILL). + * + * @author Wellington Mafra + * @see manusa/helm-java#409 + */ +@DisabledOnOs(OS.WINDOWS) // relies on POSIX SIGTERM delivery and 128+n exit codes +class HelmSignalHandlerTest { + + private static final int SIGTERM_EXIT_CODE = 128 + 15; + + @Test + void jvmExitsOnSigTermAfterInstall() throws Exception { + final String javaBin = Paths.get(System.getProperty("java.home"), "bin", "java").toString(); + final ProcessBuilder processBuilder = new ProcessBuilder( + javaBin, "-cp", System.getProperty("java.class.path"), SignalProbe.class.getName()); + processBuilder.environment().put("KUBECONFIG", "/dev/null"); + processBuilder.redirectErrorStream(true); + final Process probe = processBuilder.start(); + try { + final BufferedReader reader = + new BufferedReader(new InputStreamReader(probe.getInputStream(), StandardCharsets.UTF_8)); + boolean ready = false; + String line; + while ((line = reader.readLine()) != null) { + if (SignalProbe.READY_MARKER.equals(line)) { + ready = true; + break; + } + } + assertThat(ready) + .as("probe process should complete the native helm install") + .isTrue(); + // Process.destroy() delivers SIGTERM on POSIX systems + probe.destroy(); + assertThat(probe.waitFor(15, TimeUnit.SECONDS)) + .as("JVM should exit on SIGTERM after a native helm install (issue #409)") + .isTrue(); + assertThat(probe.exitValue()).isEqualTo(SIGTERM_EXIT_CODE); + } finally { + probe.destroyForcibly(); + } + } + + /** + * Runs in a separate JVM: performs a client-only install (reaching the native code path that + * used to call Go's {@code signal.Notify}), signals readiness, then waits to be terminated. + */ + static final class SignalProbe { + + static final String READY_MARKER = "SIGNAL_PROBE_READY"; + + public static void main(String[] args) throws Exception { + final Path tempDir = Files.createTempDirectory("helm-signal-probe"); + final Helm helm = Helm.create().withName("test").withDir(tempDir).call(); + helm.install().clientOnly().withName("test").call(); + System.out.println(READY_MARKER); + System.out.flush(); + Thread.sleep(TimeUnit.SECONDS.toMillis(60)); + // Only reached if SIGTERM was ignored (the bug) or never delivered (test infrastructure issue) + System.exit(1); + } + } +} diff --git a/native/internal/helm/install.go b/native/internal/helm/install.go index 6cafe641..979bdb68 100644 --- a/native/internal/helm/install.go +++ b/native/internal/helm/install.go @@ -30,11 +30,8 @@ import ( "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/release" "net/url" - "os" - "os/signal" "slices" "strings" - "syscall" "time" ) @@ -177,21 +174,13 @@ func install(options *InstallOptions) (*release.Release, *installOutputs, error) return nil, outputs, err } - // Create context that handles SIGINT, SIGTERM - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - // Set up channel on which to send signal notifications. - // We must use a buffered channel or risk missing the signal - // if we're not ready to receive when the signal is sent. - cSignal := make(chan os.Signal, 4) - signal.Notify(cSignal, syscall.SIGINT, syscall.SIGTERM, syscall.SIGKILL, syscall.SIGQUIT) - go func() { - <-cSignal - cancel() - }() - // Run - rel, err := client.RunWithContext(ctx, chartRequested, vals) + // Don't use signal.Notify here: in a c-shared library the first call makes the Go + // runtime permanently replace the host process' (e.g. an embedding JVM's) OS-level + // SIGINT/SIGTERM handlers, and the cancellation it enabled was never reachable by + // embedders anyway. Interrupt handling is the host application's responsibility. + // See https://github.com/manusa/helm-java/issues/409 + rel, err := client.RunWithContext(context.Background(), chartRequested, vals) return rel, outputs, err }