diff --git a/assets/js/app.js b/assets/js/app.js index 1552117dd..cf415f553 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -24,6 +24,7 @@ import MarkdownRenderer from "./markdown_renderer"; import Highlight from "./highlight"; import DragAndDrop from "./drag_and_drop"; import PasswordToggle from "./password_toggle"; +import KeyboardControl from "./keyboard_control"; import morphdomCallbacks from "./morphdom_callbacks"; import { loadUserData } from "./lib/user"; @@ -41,6 +42,7 @@ const hooks = { Highlight, DragAndDrop, PasswordToggle, + KeyboardControl, }; const csrfToken = document diff --git a/assets/js/keyboard_control/index.js b/assets/js/keyboard_control/index.js new file mode 100644 index 000000000..4ab986674 --- /dev/null +++ b/assets/js/keyboard_control/index.js @@ -0,0 +1,91 @@ +import { getAttributeOrThrow, parseBoolean } from "../lib/attribute"; +import { cancelEvent } from "../lib/utils"; + +/** + * A hook for ControlComponent to handle user keyboard interactions. + * + * Configuration: + * + * * `data-keydown-enabled` - whether keydown events should be intercepted + * + * * `data-keyup-enabled` - whether keyup events should be intercepted + * + * * `data-target` - the target to send live events to + */ +const KeyboardControl = { + mounted() { + this.props = getProps(this); + + this.handleDocumentKeyDown = (event) => { + handleDocumentKeyDown(this, event); + }; + + // We intentionally register on window rather than document, + // to intercept clicks as early on as possible, even before + // the session shortcuts + window.addEventListener("keydown", this.handleDocumentKeyDown, true); + + this.handleDocumentKeyUp = (event) => { + handleDocumentKeyUp(this, event); + }; + + window.addEventListener("keyup", this.handleDocumentKeyUp, true); + }, + + updated() { + this.props = getProps(this); + }, + + destroyed() { + window.removeEventListener("keydown", this.handleDocumentKeyDown, true); + window.removeEventListener("keyup", this.handleDocumentKeyUp, true); + }, +}; + +function getProps(hook) { + return { + isKeydownEnabled: getAttributeOrThrow( + hook.el, + "data-keydown-enabled", + parseBoolean + ), + isKeyupEnabled: getAttributeOrThrow( + hook.el, + "data-keyup-enabled", + parseBoolean + ), + target: getAttributeOrThrow(hook.el, "data-target"), + }; +} + +function handleDocumentKeyDown(hook, event) { + if (keyboardEnabled(hook)) { + cancelEvent(event); + } + + if (hook.props.isKeydownEnabled) { + if (event.repeat) { + return; + } + + const key = event.key; + hook.pushEventTo(hook.props.target, "keydown", { key }); + } +} + +function handleDocumentKeyUp(hook, event) { + if (keyboardEnabled(hook)) { + cancelEvent(event); + } + + if (hook.props.isKeyupEnabled) { + const key = event.key; + hook.pushEventTo(hook.props.target, "keyup", { key }); + } +} + +function keyboardEnabled(hook) { + return hook.props.isKeydownEnabled || hook.props.isKeyupEnabled; +} + +export default KeyboardControl; diff --git a/assets/js/lib/utils.js b/assets/js/lib/utils.js index 941faa2d0..03acf834b 100644 --- a/assets/js/lib/utils.js +++ b/assets/js/lib/utils.js @@ -127,3 +127,10 @@ export function findChildOrThrow(element, selector) { return child; } + +export function cancelEvent(event) { + // Cancel any default browser behavior. + event.preventDefault(); + // Stop event propagation (e.g. so it doesn't reach the editor). + event.stopPropagation(); +} diff --git a/assets/js/session/index.js b/assets/js/session/index.js index 23c7e36d5..91115893b 100644 --- a/assets/js/session/index.js +++ b/assets/js/session/index.js @@ -5,6 +5,7 @@ import { selectElementContent, smoothlyScrollToElement, setFavicon, + cancelEvent, } from "../lib/utils"; import { getAttributeOrDefault } from "../lib/attribute"; import KeyBuffer from "./key_buffer"; @@ -572,7 +573,7 @@ function initializeFocus(hook) { const element = document.getElementById(htmlId); if (element) { - const focusableEl = elementelement.closest("[data-focusable-id]"); + const focusableEl = element.closest("[data-focusable-id]"); if (focusableEl) { setFocusedEl(hook, focusableEl.dataset.focusableId); @@ -1067,11 +1068,4 @@ function getRuntimeInfoToggle() { return document.querySelector(`[data-element="runtime-info-toggle"]`); } -function cancelEvent(event) { - // Cancel any default browser behavior. - event.preventDefault(); - // Stop event propagation (e.g. so it doesn't reach the editor). - event.stopPropagation(); -} - export default Session; diff --git a/lib/livebook/evaluator.ex b/lib/livebook/evaluator.ex index 6026a1d0b..c3803f268 100644 --- a/lib/livebook/evaluator.ex +++ b/lib/livebook/evaluator.ex @@ -23,14 +23,12 @@ defmodule Livebook.Evaluator do @type t :: %{pid: pid(), ref: reference()} @type state :: %{ + ref: reference(), formatter: module(), io_proxy: pid(), + object_tracker: pid(), contexts: %{ref() => context()}, - initial_context: context(), - # We track the widgets rendered by every evaluation, - # so that we can kill those no longer needed - widget_pids: %{ref() => MapSet.t(pid())}, - widget_counts: %{pid() => non_neg_integer()} + initial_context: context() } @typedoc """ @@ -57,6 +55,8 @@ defmodule Livebook.Evaluator do Options: + * `object_tracker` - a PID of `Livebook.Evaluator.ObjectTracker`, required + * `formatter` - a module implementing the `Livebook.Evaluator.Formatter` behaviour, used for transforming evaluation response before it's sent to the client """ @@ -171,16 +171,18 @@ defmodule Livebook.Evaluator do end def init(opts) do + object_tracker = Keyword.fetch!(opts, :object_tracker) formatter = Keyword.get(opts, :formatter, Evaluator.IdentityFormatter) - {:ok, io_proxy} = Evaluator.IOProxy.start_link() + {:ok, io_proxy} = Evaluator.IOProxy.start_link(self(), object_tracker) - # Use the dedicated IO device as the group leader, - # so that it handles all :stdio operations. + # Use the dedicated IO device as the group leader, so that + # intercepts all :stdio requests and also handles Livebook + # specific ones Process.group_leader(self(), io_proxy) evaluator_ref = make_ref() - state = initial_state(evaluator_ref, formatter, io_proxy) + state = initial_state(evaluator_ref, formatter, io_proxy, object_tracker) evaluator = %{pid: self(), ref: evaluator_ref} :proc_lib.init_ack(evaluator) @@ -188,15 +190,14 @@ defmodule Livebook.Evaluator do loop(state) end - defp initial_state(evaluator_ref, formatter, io_proxy) do + defp initial_state(evaluator_ref, formatter, io_proxy, object_tracker) do %{ evaluator_ref: evaluator_ref, formatter: formatter, io_proxy: io_proxy, + object_tracker: object_tracker, contexts: %{}, - initial_context: initial_context(), - widget_pids: %{}, - widget_counts: %{} + initial_context: initial_context() } end @@ -221,6 +222,8 @@ defmodule Livebook.Evaluator do defp handle_cast({:evaluate_code, send_to, code, ref, prev_ref, opts}, state) do Evaluator.IOProxy.configure(state.io_proxy, send_to, ref) + Evaluator.ObjectTracker.remove_reference(state.object_tracker, {self(), ref}) + context = get_context(state, prev_ref) file = Keyword.get(opts, :file, "nofile") context = put_in(context.env.file, file) @@ -249,18 +252,12 @@ defmodule Livebook.Evaluator do metadata = %{evaluation_time_ms: evaluation_time_ms} send(send_to, {:evaluation_response, ref, output, metadata}) - widget_pids = Evaluator.IOProxy.flush_widgets(state.io_proxy) - state = track_evaluation_widgets(state, ref, widget_pids, output) - {:noreply, state} end defp handle_cast({:forget_evaluation, ref}, state) do - state = - state - |> Map.update!(:contexts, &Map.delete(&1, ref)) - |> garbage_collect_widgets(ref, []) - + state = Map.update!(state, :contexts, &Map.delete(&1, ref)) + Evaluator.ObjectTracker.remove_reference(state.object_tracker, {self(), ref}) {:noreply, state} end @@ -372,56 +369,6 @@ defmodule Livebook.Evaluator do defp internal_dictionary_key?("$" <> _), do: true defp internal_dictionary_key?(_), do: false - # Widgets - - defp track_evaluation_widgets(state, ref, widget_pids, output) do - widget_pids = - case widget_pid_from_output(output) do - {:ok, pid} -> MapSet.put(widget_pids, pid) - :error -> widget_pids - end - - garbage_collect_widgets(state, ref, widget_pids) - end - - defp garbage_collect_widgets(state, ref, widget_pids) do - prev_widget_pids = state.widget_pids[ref] || [] - - state = put_in(state.widget_pids[ref], widget_pids) - - update_in(state.widget_counts, fn counts -> - counts = - Enum.reduce(prev_widget_pids, counts, fn pid, counts -> - Map.update!(counts, pid, &(&1 - 1)) - end) - - counts = - Enum.reduce(widget_pids, counts, fn pid, counts -> - Map.update(counts, pid, 1, &(&1 + 1)) - end) - - {to_remove, to_keep} = Enum.split_with(counts, fn {_pid, count} -> count == 0 end) - - for {pid, 0} <- to_remove do - Process.exit(pid, :shutdown) - end - - Map.new(to_keep) - end) - end - - @doc """ - Checks the given output value for widget pid to track. - """ - @spec widget_pid_from_output(term()) :: {:ok, pid()} | :error - def widget_pid_from_output(output) - - def widget_pid_from_output({_type, pid}) when is_pid(pid) do - {:ok, pid} - end - - def widget_pid_from_output(_output), do: :error - defp get_execution_time_delta(started_at) do System.monotonic_time() |> Kernel.-(started_at) diff --git a/lib/livebook/evaluator/io_proxy.ex b/lib/livebook/evaluator/io_proxy.ex index f50d19c38..c34fe76a4 100644 --- a/lib/livebook/evaluator/io_proxy.ex +++ b/lib/livebook/evaluator/io_proxy.ex @@ -24,9 +24,9 @@ defmodule Livebook.Evaluator.IOProxy do Make sure to use `configure/3` to actually proxy the requests. """ - @spec start_link() :: GenServer.on_start() - def start_link(opts \\ []) do - GenServer.start_link(__MODULE__, opts) + @spec start_link(pid(), pid()) :: GenServer.on_start() + def start_link(evaluator, object_tracker) do + GenServer.start_link(__MODULE__, evaluator: evaluator, object_tracker: object_tracker) end @doc """ @@ -80,7 +80,10 @@ defmodule Livebook.Evaluator.IOProxy do ## Callbacks @impl true - def init(_opts) do + def init(opts) do + evaluator = Keyword.fetch!(opts, :evaluator) + object_tracker = Keyword.fetch!(opts, :object_tracker) + {:ok, %{ encoding: :unicode, @@ -88,8 +91,9 @@ defmodule Livebook.Evaluator.IOProxy do ref: nil, buffer: [], input_cache: %{}, - widget_pids: MapSet.new(), - token_count: 0 + token_count: 0, + evaluator: evaluator, + object_tracker: object_tracker }} end @@ -107,10 +111,6 @@ defmodule Livebook.Evaluator.IOProxy do {:reply, :ok, flush_buffer(state)} end - def handle_call(:flush_widgets, _from, state) do - {:reply, state.widget_pids, %{state | widget_pids: MapSet.new()}} - end - @impl true def handle_info({:io_request, from, reply_as, req}, state) do {reply, state} = io_request(req, state) @@ -198,13 +198,6 @@ defmodule Livebook.Evaluator.IOProxy do defp io_request({:livebook_put_output, output}, state) do state = flush_buffer(state) send(state.target, {:evaluation_output, state.ref, output}) - - state = - case Evaluator.widget_pid_from_output(output) do - {:ok, pid} -> update_in(state.widget_pids, &MapSet.put(&1, pid)) - :error -> state - end - {:ok, state} end @@ -224,6 +217,27 @@ defmodule Livebook.Evaluator.IOProxy do {token, state} end + defp io_request({:livebook_reference_object, object, pid}, state) do + # When the request comes from evaluator we want the pointer + # specific to the current evaluation. For any other process + # we only care about monitoring. + + reference = + if pid == state.evaluator do + {pid, state.ref} + else + {pid, :process} + end + + Evaluator.ObjectTracker.add_reference(state.object_tracker, object, reference) + {:ok, state} + end + + defp io_request({:livebook_monitor_object, object, destination, payload}, state) do + reply = Evaluator.ObjectTracker.monitor(state.object_tracker, object, destination, payload) + {reply, state} + end + defp io_request(_, state) do {{:error, :request}, state} end diff --git a/lib/livebook/evaluator/object_tracker.ex b/lib/livebook/evaluator/object_tracker.ex new file mode 100644 index 000000000..f9588a834 --- /dev/null +++ b/lib/livebook/evaluator/object_tracker.ex @@ -0,0 +1,150 @@ +defmodule Livebook.Evaluator.ObjectTracker do + @moduledoc false + + # This module is an abstraction for tracking objects, + # references to them and garbage collection. + # + # Every object is identified by an arbitrary unique term. + # Processes can reference those objects by adding a pair + # of `{pid, scope}`, scope is an optional additinal term + # distinguishing the reference. + # + # Each reference can be released either manually by calling + # `remove_reference/2` or automatically when the pointing + # process terminates. + # + # When all references for the given object are removed, + # all messages scheduled with `monitor/3` are sent. + + use GenServer + + @type state :: %{ + objects: %{ + object() => %{ + references: list(object_reference()), + monitors: list(monitor()) + } + } + } + + @typedoc """ + Arbitrary term identifying an object. + """ + @type object :: term() + + @typedoc """ + Reference to an object with an optional scope. + """ + @type object_reference :: {process :: pid(), scope :: term()} + + @typedoc """ + Scheduled message to be sent when an object is released. + """ + @type monitor :: {Process.dest(), payload :: term()} + + @doc """ + Starts a new object tracker. + """ + @spec start_link(keyword()) :: GenServer.on_start() + def start_link(opts \\ []) do + GenServer.start_link(__MODULE__, opts) + end + + @doc """ + Adds a reference to the given object. + """ + @spec add_reference(pid(), object(), object_reference()) :: :ok + def add_reference(object_tracker, object, reference) do + GenServer.cast(object_tracker, {:add_reference, object, reference}) + end + + @doc """ + Removes the given reference from all objects it is attached to. + """ + @spec remove_reference(pid(), object_reference()) :: :ok + def remove_reference(object_tracker, reference) do + GenServer.cast(object_tracker, {:remove_reference, reference}) + end + + @doc """ + Schedules `payload` to be send to `destination` when the object + is released. + """ + @spec monitor(pid(), object(), Process.dest(), term()) :: :ok | {:error, :bad_object} + def monitor(object_tracker, object, destination, payload) do + GenServer.call(object_tracker, {:monitor, object, destination, payload}) + end + + @impl true + def init(_opts) do + {:ok, %{objects: %{}}} + end + + @impl true + def handle_cast({:add_reference, object, reference}, state) do + {parent, _scope} = reference + Process.monitor(parent) + + state = + if state.objects[object] do + update_in(state.objects[object].references, fn references -> + if reference in references, do: references, else: [reference | references] + end) + else + put_in(state.objects[object], %{references: [reference], monitors: []}) + end + + {:noreply, state} + end + + def handle_cast({:remove_reference, reference}, state) do + state = update_references(state, fn references -> List.delete(references, reference) end) + + {:noreply, garbage_collect(state)} + end + + @impl true + def handle_call({:monitor, object, destination, payload}, _from, state) do + monitor = {destination, payload} + + if state.objects[object] do + state = + update_in(state.objects[object].monitors, fn monitors -> + if monitor in monitors, do: monitors, else: [monitor | monitors] + end) + + {:reply, :ok, garbage_collect(state)} + else + {:reply, {:error, :bad_object}, state} + end + end + + @impl true + def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do + state = + update_references(state, fn references -> + Enum.reject(references, &match?({^pid, _}, &1)) + end) + + {:noreply, garbage_collect(state)} + end + + # Updates references for every object with the given function + defp update_references(state, fun) do + update_in(state.objects, fn objects -> + for {object, %{references: references} = info} <- objects, into: %{} do + {object, %{info | references: fun.(references)}} + end + end) + end + + defp garbage_collect(state) do + {to_release, objects} = Enum.split_with(state.objects, &match?({_, %{references: []}}, &1)) + + for {_, %{monitors: monitors}} <- to_release, {dest, payload} <- monitors do + send(dest, payload) + end + + %{state | objects: Map.new(objects)} + end +end diff --git a/lib/livebook/runtime/erl_dist.ex b/lib/livebook/runtime/erl_dist.ex index 300c8f0a0..1f96baa19 100644 --- a/lib/livebook/runtime/erl_dist.ex +++ b/lib/livebook/runtime/erl_dist.ex @@ -23,6 +23,7 @@ defmodule Livebook.Runtime.ErlDist do @required_modules [ Livebook.Evaluator, Livebook.Evaluator.IOProxy, + Livebook.Evaluator.ObjectTracker, Livebook.Evaluator.DefaultFormatter, Livebook.Intellisense, Livebook.Intellisense.IdentifierMatcher, diff --git a/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex b/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex index 7d1f4d175..c1233a58e 100644 --- a/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex +++ b/lib/livebook/runtime/erl_dist/evaluator_supervisor.ex @@ -20,11 +20,11 @@ defmodule Livebook.Runtime.ErlDist.EvaluatorSupervisor do @doc """ Spawns a new evaluator. """ - @spec start_evaluator(pid()) :: {:ok, Evaluator.t()} | {:error, any()} - def start_evaluator(supervisor) do + @spec start_evaluator(pid(), pid()) :: {:ok, Evaluator.t()} | {:error, any()} + def start_evaluator(supervisor, object_tracker) do case DynamicSupervisor.start_child( supervisor, - {Evaluator, [formatter: Evaluator.DefaultFormatter]} + {Evaluator, [formatter: Evaluator.DefaultFormatter, object_tracker: object_tracker]} ) do {:ok, _pid, evaluator} -> {:ok, evaluator} {:error, reason} -> {:error, reason} diff --git a/lib/livebook/runtime/erl_dist/runtime_server.ex b/lib/livebook/runtime/erl_dist/runtime_server.ex index 4a6ac33d9..20f316171 100644 --- a/lib/livebook/runtime/erl_dist/runtime_server.ex +++ b/lib/livebook/runtime/erl_dist/runtime_server.ex @@ -115,13 +115,15 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do {:ok, evaluator_supervisor} = ErlDist.EvaluatorSupervisor.start_link() {:ok, completion_supervisor} = Task.Supervisor.start_link() + {:ok, object_tracker} = Livebook.Evaluator.ObjectTracker.start_link() {:ok, %{ owner: nil, evaluators: %{}, evaluator_supervisor: evaluator_supervisor, - completion_supervisor: completion_supervisor + completion_supervisor: completion_supervisor, + object_tracker: object_tracker }} end @@ -234,7 +236,12 @@ defmodule Livebook.Runtime.ErlDist.RuntimeServer do if Map.has_key?(state.evaluators, container_ref) do state else - {:ok, evaluator} = ErlDist.EvaluatorSupervisor.start_evaluator(state.evaluator_supervisor) + {:ok, evaluator} = + ErlDist.EvaluatorSupervisor.start_evaluator( + state.evaluator_supervisor, + state.object_tracker + ) + Process.monitor(evaluator.pid) %{state | evaluators: Map.put(state.evaluators, container_ref, evaluator)} end diff --git a/lib/livebook/session.ex b/lib/livebook/session.ex index 098fb77c3..9183b5e8d 100644 --- a/lib/livebook/session.ex +++ b/lib/livebook/session.ex @@ -619,7 +619,7 @@ defmodule Livebook.Session do maybe_save_notebook_sync(state) broadcast_message(state.session_id, :session_closed) - {:stop, :shutdown, state} + {:stop, :normal, state} end @impl true diff --git a/lib/livebook_web/live/output.ex b/lib/livebook_web/live/output.ex index 2a07ef31b..c8a470a32 100644 --- a/lib/livebook_web/live/output.ex +++ b/lib/livebook_web/live/output.ex @@ -113,6 +113,10 @@ defmodule LivebookWeb.Output do ) end + defp render_output({:control, attrs}, %{id: id}) do + live_component(LivebookWeb.Output.ControlComponent, id: id, attrs: attrs) + end + defp render_output({:error, formatted, :runtime_restart_required}, %{runtime: runtime}) when runtime != nil do assigns = %{formatted: formatted, is_standalone: Livebook.Runtime.standalone?(runtime)} diff --git a/lib/livebook_web/live/output/control_component.ex b/lib/livebook_web/live/output/control_component.ex new file mode 100644 index 000000000..2728e93ff --- /dev/null +++ b/lib/livebook_web/live/output/control_component.ex @@ -0,0 +1,81 @@ +defmodule LivebookWeb.Output.ControlComponent do + use LivebookWeb, :live_component + + @impl true + def mount(socket) do + {:ok, assign(socket, keyboard_enabled: false)} + end + + @impl true + def render(%{attrs: %{type: :keyboard}} = assigns) do + ~H""" +