From b172f5ef329665d66c1b681d8daa611d260fb137 Mon Sep 17 00:00:00 2001 From: Jakub Holy Date: Wed, 24 Jul 2024 23:46:04 +0200 Subject: [PATCH] Fix #79 Add core/restart Issue: We want to be able to restart the kernel with new aliases, without restarting the REPL. Fix: Make stop not only terminate the kernel, but also clean up everything so that start will start from scratch. Also, make it not fail if the kernel is not actually started yet. Also make terminate do call .close as docs suggest we should, and make start retry for a few times with a growing delay - since close+terminate make take a while to finish and may throw before that. --- src/wolframite/base/convert.clj | 10 ++-- src/wolframite/core.clj | 21 ++++++--- src/wolframite/impl/jlink_instance.clj | 4 +- src/wolframite/impl/jlink_proto_impl.clj | 59 +++++++++++++++--------- test/wolframite/core_test.clj | 19 +++++++- 5 files changed, 78 insertions(+), 35 deletions(-) diff --git a/src/wolframite/base/convert.clj b/src/wolframite/base/convert.clj index c964546..05ab7d4 100644 --- a/src/wolframite/base/convert.clj +++ b/src/wolframite/base/convert.clj @@ -115,9 +115,13 @@ ;(let [s (str-utils/replace (str sym) #"\|(.*?)\|" #(str "\\\\[" (second %) "]"))] ) (let [s (str sym)] (if (re-find #"[^a-zA-Z0-9$\/]" s) - (throw (Exception. (str "Symbols passed to Mathematica must be alphanumeric (apart from forward slashes and dollar signs). Passed: " - s - "Known aliases:" (keys aliases)))) + (throw (ex-info (str "Unsupported symbol / unknown alias: Symbols passed to Mathematica must be alphanumeric" + " (apart from forward slashes and dollar signs). Other symbols may" + " only be used if there is defined a Wolframite alias for them." + " Passed: " s + " Known aliases: " (or (-> aliases keys sort seq) "N/A")) + {:unknown-symbol s + :known-symbols (keys aliases)})) (proto/expr (jlink-instance/get) :Expr/SYMBOL s))))))) (defn- convert-non-simple-list [elms opts] diff --git a/src/wolframite/core.clj b/src/wolframite/core.clj index ddbd11d..01b2db7 100644 --- a/src/wolframite/core.clj +++ b/src/wolframite/core.clj @@ -86,13 +86,6 @@ (->> (kernel-link-opts init-opts) (proto/create-kernel-link jlink-impl)))) -(defn stop - "Sends a request to the kernel to shut down. - - See https://reference.wolfram.com/language/JLink/ref/java/com/wolfram/jlink/KernelLink.html#terminateKernel()" - [] - (proto/terminate-kernel! (jlink-instance/get))) - (defn- unqualify [form] (walk/postwalk (fn [form] (if (qualified-symbol? form) @@ -166,6 +159,20 @@ nil) nil)) +(defn stop + "Sends a request to the kernel to shut down. + + See https://reference.wolfram.com/language/JLink/ref/java/com/wolfram/jlink/KernelLink.html#terminateKernel()" + [] + (some-> (jlink-instance/get) (proto/terminate-kernel!)) + (jlink-instance/reset!) + (reset! kernel-link-atom nil)) + +(defn restart + "Same as calling [[stop]] then [[start]]" + ([] (stop) (start)) + ([opts] (stop) (start opts))) + (defn eval "Evaluate the given Wolfram expression (a string, or a Clojure data) and return the result as Clojure data. diff --git a/src/wolframite/impl/jlink_instance.clj b/src/wolframite/impl/jlink_instance.clj index 51563a2..c950391 100644 --- a/src/wolframite/impl/jlink_instance.clj +++ b/src/wolframite/impl/jlink_instance.clj @@ -1,9 +1,11 @@ (ns wolframite.impl.jlink-instance (:require [wolframite.impl.protocols :as proto]) (:import [wolframite.impl.protocols JLink]) - (:refer-clojure :exclude [get])) + (:refer-clojure :exclude [get reset!])) ;; The actual impl. of the JLink interface, set at runtime (defonce jlink-instance (atom nil)) (defn ^JLink get [] (deref jlink-instance)) + +(defn reset! [] (clojure.core/reset! jlink-instance nil)) diff --git a/src/wolframite/impl/jlink_proto_impl.clj b/src/wolframite/impl/jlink_proto_impl.clj index d1e4af2..de738ca 100644 --- a/src/wolframite/impl/jlink_proto_impl.clj +++ b/src/wolframite/impl/jlink_proto_impl.clj @@ -57,30 +57,43 @@ (defrecord JLinkImpl [opts kernel-link-atom] proto/JLink (create-kernel-link [_this kernel-link-opts] - (try (let [opts-array (into-array String kernel-link-opts) - kernel-link - (->> (doto (MathLinkFactory/createKernelLink ^"[Ljava.lang.String;" opts-array) - (.discardAnswer)) - (reset! kernel-link-atom))] - ;(.getError kernel-link) (.getErrorMessage kernel-link) - kernel-link) - (catch MathLinkException e - (if (= (ex-message e) "MathLink connection was lost.") - (throw (ex-info (str "MathLink connection was lost. Perhaps you need to activate Mathematica first," - " you are trying to start multiple concurrent connections (from separate REPLs)," - " or there is some other issue and you need to retry, or restart and retry...") - {:kernel-link-opts (cond-> kernel-link-opts - (array? kernel-link-opts) - vec) - :cause e})) - (throw e))) - (catch Exception e - (throw (ex-info (str "Failed to start a Math/Wolfram Kernel process: " - (ex-message e) - " Verify the settings are correct: `" kernel-link-opts "`") - {:kernel-opts kernel-link-opts}))))) + (loop [attempts 3, wait-ms 10, orig-err nil] + ;; Sometimes, starting a link may fail b/c the previous one + ;; has not been shut down properly + (let [res + (try (let [opts-array (into-array String kernel-link-opts) + kernel-link + (->> (doto (MathLinkFactory/createKernelLink ^"[Ljava.lang.String;" opts-array) + (.discardAnswer)) + (reset! kernel-link-atom))] + ;(.getError kernel-link) (.getErrorMessage kernel-link) + kernel-link) + (catch MathLinkException e + (if (= (ex-message e) "MathLink connection was lost.") + (throw (ex-info (str "MathLink connection was lost. Perhaps you need to activate Mathematica first," + " you are trying to start multiple concurrent connections (from separate REPLs)," + " or there is some other issue and you need to retry, or restart and retry...") + {:kernel-link-opts (cond-> kernel-link-opts + (array? kernel-link-opts) + vec) + :cause e})) + (throw e))) + (catch Exception e + (throw (ex-info (str "Failed to start a Math/Wolfram Kernel process: " + (ex-message e) + " Verify the settings are correct: `" kernel-link-opts "`") + {:kernel-opts kernel-link-opts}))))] + (if (instance? Exception res) + (if (pos? attempts) + (do (Thread/sleep wait-ms) + (recur (dec attempts) (* 3 wait-ms) (or orig-err res))) + (throw orig-err)) + res)))) (terminate-kernel! [_this] - (.terminateKernel ^KernelLink @kernel-link-atom) + ;; BEWARE: it is not absolutely guaranteed that the kernel will die immediately + (doto ^KernelLink @kernel-link-atom + (.terminateKernel) + (.close)) (reset! kernel-link-atom nil)) (expr [_this primitive-or-exprs] (make-expr primitive-or-exprs)) diff --git a/test/wolframite/core_test.clj b/test/wolframite/core_test.clj index ccef639..c3cab00 100644 --- a/test/wolframite/core_test.clj +++ b/test/wolframite/core_test.clj @@ -2,7 +2,8 @@ (:require [clojure.test :refer [deftest testing is]] [wolframite.core :as wl] [wolframite.impl.wolfram-syms.wolfram-syms :as wolfram-syms] - [wolframite.wolfram :as w])) + [wolframite.wolfram :as w]) + (:import (clojure.lang ExceptionInfo))) ;; * Basic Sanity Checks @@ -43,6 +44,22 @@ (eval '(-> #'w2/Plus meta :doc))) "Interned vars have docstrings")) +(deftest restart + (testing "first set of aliases" + (wl/restart {:aliases '{** Power}}) + (is (= 8 + (wl/eval '(** 2 3))) + "** is an alias for Power (and 2^3 is 8)")) + (testing "restart & another aliases" + (wl/restart {:aliases '{pow Power}}) + (is (thrown-with-msg? ExceptionInfo + #"Unsupported symbol / unknown alias" + (wl/eval '(** 2 3))) + "** is not known anymore") + (is (= 8 + (wl/eval '(pow 2 3))) + "Now, `pow` is an alias for Power (and 2^3 is 8)"))) + (deftest bug-fixes (wl/start) (testing "#76 double eval of ->"