From 64e856a48ceed8ab478e7404686400d796a266d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Mottelet?= Date: Thu, 24 Sep 2026 10:42:05 +0200 Subject: [PATCH] Fix ~30s hang on unclosed blocks (function/if/for/... with no end) Running a cell that leaves a block unclosed -- function x=f(y) with no endfunction, an if/for/while/select/try with no matching end -- left Scilab sitting at its continuation prompt waiting for the rest of the block, and the kernel just hung. metakernel's REPLWrapper does detect this (fast, well under a second) and has a built-in recovery: send Ctrl-C, wait for a normal prompt, then raise a clean, catchable ValueError. But scilab-adv-cli does not respond to SIGINT while waiting for more input, so that recovery wait always ran out its full budget (pexpect's default of 30s) before giving up -- so every incomplete cell cost a good 30 seconds before showing a generic "Timed out" error, though the session did eventually recover on its own (confirmed: the next cell executes normally afterward). Fixed with a small REPLWrapper subclass (_ScilabREPLWrapper) that overrides just the continuation-prompt recovery step. A bare "end" reliably closes any Scilab block type (function/if/for/while/...) and returns to the top-level prompt in well under a second -- verified directly against a live Scilab process for each block type. Nested unclosed blocks (e.g. an unclosed if containing an unclosed for) need one "end" per level, so it's sent repeatedly (capped at 50 attempts) until a normal prompt reappears, matching what a user closing the blocks by hand would have to type anyway. Everything else about metakernel's flow is unchanged -- the "soft continuation" empty-line retry, the eventual ValueError, and its message ("Continuation prompt found - input was incomplete:\n") are all metakernel's own, already well-suited to surfacing a clear error to the user; this only fixes the part that was actually broken for Scilab specifically. Verified against a live kernel: unclosed function/if/for now each report the error in ~0.2-0.3s (down from 30s+), a nested unclosed if+for needs and gets two "end"s and also recovers in ~0.3s, normal complete multi-line blocks (if/end, function/endfunction, for/end) are unaffected and execute correctly, and the session remains fully functional for subsequent cells in every case. Also includes the same pre-existing ruff cleanup as #58/#59 (this branch was cut from master, which still has that debt). Co-Authored-By: Claude Sonnet 5 --- scilab_kernel/check.py | 13 ++-- scilab_kernel/kernel.py | 95 ++++++++++++++++++++---------- scilab_kernel/magics/plot_magic.py | 2 + test_scilab_kernel.py | 6 +- 4 files changed, 77 insertions(+), 39 deletions(-) diff --git a/scilab_kernel/check.py b/scilab_kernel/check.py index d01362e..798bde5 100644 --- a/scilab_kernel/check.py +++ b/scilab_kernel/check.py @@ -1,18 +1,19 @@ import sys + from metakernel import __version__ as mversion + from . import __version__ from .kernel import ScilabKernel - if __name__ == "__main__": - print('Scilab kernel v%s' % __version__) - print('Metakernel v%s' % mversion) - print('Python v%s' % sys.version) - print('Python path: %s' % sys.executable) + print(f'Scilab kernel v{__version__}') + print(f'Metakernel v{mversion}') + print(f'Python v{sys.version}') + print(f'Python path: {sys.executable}') print('\nConnecting to Scilab...') try: s = ScilabKernel() print('Scilab connection established') print(s.banner) - except Exception as e: + except Exception as e: # noqa: BLE001 -- diagnostic script: report any failure, don't crash with a raw traceback print(e) diff --git a/scilab_kernel/kernel.py b/scilab_kernel/kernel.py index c07315b..b2f022a 100644 --- a/scilab_kernel/kernel.py +++ b/scilab_kernel/kernel.py @@ -1,26 +1,58 @@ -from __future__ import print_function, absolute_import import codecs +import importlib import json import os +import platform import re import shutil +import subprocess import sys -import platform import tempfile -import importlib -import subprocess +from typing import ClassVar + if importlib.util.find_spec('winreg'): import winreg from xml.dom import minidom +from xml.parsers.expat import ExpatError +from IPython.display import SVG, Image from metakernel import MetaKernel, ProcessMetaKernel, REPLWrapper, pexpect from metakernel.pexpect import which -from IPython.display import Image, SVG from . import __version__ +class _ScilabREPLWrapper(REPLWrapper): + """A :class:`REPLWrapper` that knows how to escape Scilab's continuation + prompt. + + When a cell leaves a block unclosed (``function x=f(y)`` with no + ``endfunction``, an ``if``/``for``/``while``/... with no matching + ``end``, ...), Scilab drops into a continuation prompt waiting for the + rest of the block. REPLWrapper's own recovery for this sends Ctrl-C and + waits (up to 30s) for a normal prompt to come back -- but + ``scilab-adv-cli`` does not respond to SIGINT while waiting for more + input, so that 30s is always spent in full, and the user just sees a + generic "Timed out" error after a long pause. + + A bare "end" closes any Scilab block type and returns to the top-level + prompt in well under a second; nested unclosed blocks need one "end" + per level, so it is sent repeatedly until a normal prompt reappears. + """ + + _MAX_END_ATTEMPTS = 50 + + def interrupt(self, continuation=False): + if not continuation: + return super().interrupt(continuation=continuation) + for _ in range(self._MAX_END_ATTEMPTS): + self.sendline("end") + if self._expect_prompt(timeout=-1) == 0: + break + return self.child.before + + def get_kernel_json(): """Get the kernel json for the kernel. """ @@ -37,7 +69,7 @@ class ScilabKernel(ProcessMetaKernel): implementation_version = __version__, language = 'scilab' language_version = __version__, - language_info = { + language_info: ClassVar[dict] = { 'name': 'scilab', 'file_extension': '.sci', "mimetype": "text/x-scilab", @@ -106,7 +138,7 @@ def _detect_executable(self): # read the windows registry if os.name == 'nt': try: - with winreg.OpenKey(winreg.HKEY_CLASSES_ROOT, "Scilab5.sce\shell\open\command") as key: + with winreg.OpenKey(winreg.HKEY_CLASSES_ROOT, r"Scilab5.sce\shell\open\command") as key: cmd : str = winreg.EnumValue(key, 0)[1] executable = cmd.split(r'"')[1].replace("wscilex.exe", "wscilex-cli.exe") self.log.warning('Windows registry binary: ' + executable) @@ -116,9 +148,9 @@ def _detect_executable(self): # detect macOS bundle if platform.system() == 'Darwin': - process = subprocess.run(['mdfind', '-onlyin', '/Applications', 'kMDItemCFBundleIdentifier=org.scilab.modules.jvm.Scilab'], - stdout=subprocess.PIPE, - universal_newlines=True) + process = subprocess.run(['mdfind', '-onlyin', '/Applications', 'kMDItemCFBundleIdentifier=org.scilab.modules.jvm.Scilab'], + stdout=subprocess.PIPE, + text=True, check=False) bundles = process.stdout if len(bundles) > 0: executable = bundles.split('\n', 1)[0] + "/Contents/bin/scilab-adv-cli" @@ -145,7 +177,7 @@ def makeWrapper(self): orig_prompt = r'-[0-9]*->' prompt_cmd = None change_prompt = None - continuation_prompt = ' \>' + continuation_prompt = r' \>' self._first = True if os.name == 'nt': prompt_cmd = 'printf("-->")' @@ -157,7 +189,7 @@ def makeWrapper(self): echo=echo, codec_errors="ignore", encoding="utf-8") - wrapper = REPLWrapper(child, orig_prompt, change_prompt, + wrapper = _ScilabREPLWrapper(child, orig_prompt, change_prompt, prompt_emit_cmd=prompt_cmd, echo=echo, continuation_prompt_regex=continuation_prompt) @@ -166,7 +198,7 @@ def makeWrapper(self): def Write(self, message): clean_msg = message.strip("\n\r\t") - super(ScilabKernel, self).Write(clean_msg) + super().Write(clean_msg) def Print(self, text): text = str(text).strip('\x1b[0m').replace('\u0008', '').strip() @@ -174,7 +206,7 @@ def Print(self, text): if (not line.startswith(chr(27)))] text = '\n'.join(text) if text: - super(ScilabKernel, self).Print(text) + super().Print(text) def do_execute_direct(self, code, silent=False): if self._first: @@ -182,7 +214,7 @@ def do_execute_direct(self, code, silent=False): self.handle_plot_settings() setup = self._setup.strip() self.do_execute_direct(setup, True) - resp = super(ScilabKernel, self).do_execute_direct(code, silent=silent) + resp = super().do_execute_direct(code, silent=silent) if silent: return resp if self.plot_settings.get('backend', None) == 'inline': @@ -198,17 +230,18 @@ def get_kernel_help_on(self, info, level=0, none_on_fail=False): return None else: return "" - self.do_execute_direct('help %s' % obj, True) + self.do_execute_direct(f'help {obj}', True) def do_shutdown(self, restart): self.wrapper.sendline('quit') - super(ScilabKernel, self).do_shutdown(restart) + super().do_shutdown(restart) def get_completions(self, info): """ Get completions from kernel based on info dict. """ - cmd = 'completion("%s")' % info['obj'] + obj = info['obj'] + cmd = f'completion("{obj}")' output = self.do_execute_direct(cmd, True) if not output: return [] @@ -238,17 +271,17 @@ def handle_plot_settings(self): try: width, height = settings['size'].split(',') width, height = int(width), int(height) - except Exception as e: - self.Error('Error setting plot settings: %s' % e) + except (ValueError, AttributeError) as e: + self.Error(f'Error setting plot settings: {e}') - cmds.append('h.figure_size = [%s,%s];' % (width, height)) - cmds.append('h.axes_size = [%s * 0.98, %s * 0.8];' % (width, height)) + cmds.append(f'h.figure_size = [{width},{height}];') + cmds.append(f'h.axes_size = [{width} * 0.98, {height} * 0.8];') if settings['backend'] == 'inline': cmds.append('h.visible = "off";') else: cmds.append('h.visible = "on";') - super(ScilabKernel, self).do_execute_direct('\n'.join(cmds), True) + super().do_execute_direct('\n'.join(cmds), True) def make_figures(self, plot_dir=None): """Create figures for the current figures. @@ -267,7 +300,7 @@ def make_figures(self, plot_dir=None): plot_format = self._plot_fmt.lower() make_figs = '_make_figures("%s", "%s");' make_figs = make_figs % (plot_dir, plot_format) - super(ScilabKernel, self).do_execute_direct(make_figs, True) + super().do_execute_direct(make_figs, True) return plot_dir def extract_figures(self, plot_dir): @@ -291,7 +324,7 @@ def extract_figures(self, plot_dir): if self.error_handler: self.error_handler(e) else: - raise e + raise return images def _handle_svg(self, filename): @@ -305,14 +338,14 @@ def _handle_svg(self, filename): im = SVG(data=data) try: im.data = self._fix_svg_size(im.data) - except Exception: - pass + except (ValueError, ExpatError) as e: + self.log.debug(f'Could not resize SVG (unexpected shape from GnuPlot?): {e}') try: settings = self.plot_settings if settings['antialiasing']: im.data = self._fix_svg_antialiasing(im.data) - except Exception: - pass + except (ValueError, ExpatError) as e: + self.log.debug(f'Could not adjust SVG antialiasing (unexpected shape from GnuPlot?): {e}') return im def _fix_svg_size(self, data): @@ -340,8 +373,8 @@ def _fix_svg_size(self, data): width = width * settings['height'] / height height = settings['height'] - svg.setAttribute('width', '%dpx' % width) - svg.setAttribute('height', '%dpx' % height) + svg.setAttribute('width', f'{int(width)}px') + svg.setAttribute('height', f'{int(height)}px') return svg.toxml() def _fix_svg_antialiasing(self, data): diff --git a/scilab_kernel/magics/plot_magic.py b/scilab_kernel/magics/plot_magic.py index 3019afd..7bedef9 100644 --- a/scilab_kernel/magics/plot_magic.py +++ b/scilab_kernel/magics/plot_magic.py @@ -1,4 +1,6 @@ from metakernel import Magic, option + + class ScilabPlotMagic(Magic): @option( diff --git a/test_scilab_kernel.py b/test_scilab_kernel.py index 7747ad8..26338d7 100644 --- a/test_scilab_kernel.py +++ b/test_scilab_kernel.py @@ -1,6 +1,8 @@ """Example use of jupyter_kernel_test, with tests for IPython.""" import unittest +from typing import ClassVar + import jupyter_kernel_test as jkt @@ -11,12 +13,12 @@ class ScilabKernelTests(jkt.KernelTests): code_hello_world = "disp('hello, world')" - code_display_data = [ + code_display_data: ClassVar[list] = [ {'code': '%plot -f png\nplot([1,2,3])', 'mime': 'image/png'}, {'code': '%plot -f svg\nplot([1,2,3])', 'mime': 'image/svg+xml'} ] - completion_samples = [ + completion_samples: ClassVar[list] = [ { 'text': 'one', 'matches': {'ones'},