Skip to content

Commit 21b845a

Browse files
HwangRockHwangRock
authored
[ZEPPELIN-6561] Restore default SIGINT handler so python paragraph cancel works under daemon launch
### What is this PR for? Cancelling a running `%python` paragraph has no effect when Zeppelin is started via `zeppelin-daemon.sh`: the user code runs to completion and the result is recorded as SUCCESS while the job status becomes ABORT. The cancel plumbing itself works. `cancel()` in `PythonInterpreter` sends SIGINT to the correct python pid, and the interpreter log shows it. The problem is signal disposition inheritance: `zeppelin-daemon.sh` starts the server with `nohup ... &` from a non-interactive shell, so per POSIX the whole process chain (ZeppelinServer JVM, interpreter JVM, python) inherits SIGINT=SIG_IGN, and CPython keeps SIGINT ignored instead of installing the KeyboardInterrupt handler when it starts with the signal already ignored. The SIGINT sent by `cancel()` is then a no-op. Running `signal.getsignal(signal.SIGINT)` inside an affected interpreter prints `Handlers.SIG_IGN`. This cannot be fixed in the shell scripts, since POSIX forbids a non-interactive shell from resetting a signal that was ignored on entry. The fix restores the default SIGINT handler at the top of `zeppelin_python.py` when the inherited disposition is SIG_IGN. Starting Zeppelin in the foreground with `bin/zeppelin.sh` was never affected, which is why cancellation appears to work in some environments and not in others. ### What type of PR is it? Bug Fix ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-6561 ### How should this be tested? * Automated: `testSigintDefaultHandlerRestoredWhenInheritedIgnored` in `PythonInterpreterTest` launches the interpreter through a shell wrapper that ignores SIGINT before exec'ing python, reproducing the disposition of a daemon launch, and asserts the default handler is restored inside the interpreter process. Without the fix the assertion fails with `Handlers.SIG_IGN`. Unlike the disabled `testCancelIntp`, it does not depend on timing. * Manual: start Zeppelin with `bin/zeppelin-daemon.sh start`, run a `%python` paragraph such as `for i in range(1, 50): print(i); time.sleep(0.5)`, and cancel it a few seconds in. Before the fix it runs to 49 and stores SUCCESS. After the fix it stops immediately with a KeyboardInterrupt traceback and ERROR. ### Screenshots (if appropriate) #### Before https://github.com/user-attachments/assets/b9c6f5b4-24f6-4eb9-9019-37dab52526f0 #### After https://github.com/user-attachments/assets/7b35d549-d97e-45cb-9d95-4ae5c543e0c0 ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Closes #5346 from HwangRock/ZEPPELIN-6561. Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>
1 parent d8c43cb commit 21b845a

2 files changed

Lines changed: 51 additions & 1 deletion

File tree

‎python/src/main/resources/python/zeppelin_python.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,20 @@
1515
# limitations under the License.
1616
#
1717

18-
import os, sys, traceback, json, re
18+
import os, signal, sys, traceback, json, re
1919

2020
from py4j.java_gateway import java_import, JavaGateway, GatewayClient
2121
from py4j.protocol import Py4JJavaError
2222

2323
import ast
2424

25+
# When Zeppelin is started via zeppelin-daemon.sh (nohup ... &), this process
26+
# inherits SIGINT=SIG_IGN and CPython keeps it ignored instead of installing the
27+
# KeyboardInterrupt handler, so PythonInterpreter.cancel()'s SIGINT would be a
28+
# no-op. Restore the default handler to keep paragraph cancellation working.
29+
if signal.getsignal(signal.SIGINT) == signal.SIG_IGN:
30+
signal.signal(signal.SIGINT, signal.default_int_handler)
31+
2532
class Logger(object):
2633
def __init__(self):
2734
pass

‎python/src/test/java/org/apache/zeppelin/python/PythonInterpreterTest.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.zeppelin.interpreter.InterpreterException;
2525
import org.apache.zeppelin.interpreter.InterpreterGroup;
2626
import org.apache.zeppelin.interpreter.InterpreterResult;
27+
import org.apache.zeppelin.interpreter.InterpreterResultMessage;
2728
import org.apache.zeppelin.interpreter.LazyOpenInterpreter;
2829
import org.junit.jupiter.api.AfterEach;
2930
import org.junit.jupiter.api.BeforeEach;
@@ -35,8 +36,12 @@
3536
import static org.junit.jupiter.api.Assertions.assertTrue;
3637
import static org.junit.jupiter.api.Assertions.fail;
3738

39+
import java.io.File;
3840
import java.io.IOException;
41+
import java.nio.file.Files;
42+
import java.util.Arrays;
3943
import java.util.LinkedList;
44+
import java.util.List;
4045
import java.util.Properties;
4146
import java.util.concurrent.TimeoutException;
4247
import java.util.regex.Matcher;
@@ -174,4 +179,42 @@ public void testFailtoLaunchPythonProcess() throws InterpreterException {
174179
assertTrue(stacktrace.contains("No such file or directory"), stacktrace);
175180
}
176181
}
182+
183+
@Test
184+
public void testSigintDefaultHandlerRestoredWhenInheritedIgnored()
185+
throws IOException, InterpreterException {
186+
tearDown();
187+
188+
File wrapper = File.createTempFile("python-sigint-wrapper", ".sh");
189+
wrapper.deleteOnExit();
190+
Files.write(wrapper.toPath(), Arrays.asList(
191+
"#!/bin/sh",
192+
"trap '' INT",
193+
"exec python \"$@\""));
194+
wrapper.setExecutable(true);
195+
196+
intpGroup = new InterpreterGroup();
197+
198+
Properties properties = new Properties();
199+
properties.setProperty("zeppelin.python", wrapper.getAbsolutePath());
200+
properties.setProperty("zeppelin.python.useIPython", "false");
201+
properties.setProperty("zeppelin.python.gatewayserver_address", "127.0.0.1");
202+
203+
interpreter = new LazyOpenInterpreter(new PythonInterpreter(properties));
204+
205+
intpGroup.put("note", new LinkedList<Interpreter>());
206+
intpGroup.get("note").add(interpreter);
207+
interpreter.setInterpreterGroup(intpGroup);
208+
209+
InterpreterContext.set(getInterpreterContext());
210+
211+
InterpreterContext context = getInterpreterContext();
212+
InterpreterResult result = interpreter.interpret(
213+
"import signal\nprint(signal.getsignal(signal.SIGINT))", context);
214+
assertEquals(InterpreterResult.Code.SUCCESS, result.code());
215+
List<InterpreterResultMessage> interpreterResultMessages =
216+
context.out.toInterpreterResultMessage();
217+
String output = interpreterResultMessages.get(0).getData();
218+
assertTrue(output.contains("default_int_handler"), output);
219+
}
177220
}

0 commit comments

Comments
 (0)