From: Emilie Burgun <emilie@shadamethyst.xyz>
Date: Mon, 3 Feb 2025 13:58:04 +0000 (+0100)
Subject: Fix stream realiasing possibly shadowing other streams.
X-Git-Tag: v0.10.0~74^2~2
X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=7f2ce57ba77b68fe85fcc25bbf94f9450ad9cc9d;p=scryer-prolog.git

Fix stream realiasing possibly shadowing other streams.
---

diff --git a/src/machine/machine_indices.rs b/src/machine/machine_indices.rs
index 4c849068..e2d7b248 100644
--- a/src/machine/machine_indices.rs
+++ b/src/machine/machine_indices.rs
@@ -484,6 +484,8 @@ impl IndexStore {
     pub(crate) fn remove_stream(&mut self, stream: Stream) {
         if let Some(alias) = stream.options().get_alias() {
             debug_assert_eq!(self.stream_aliases.get(&alias), Some(&stream));
+            assert!(!is_protected_alias(alias));
+
             self.stream_aliases.swap_remove(&alias);
         }
         self.streams.remove(&stream);
@@ -503,6 +505,18 @@ impl IndexStore {
         callback(options);
 
         if options.get_alias() != prev_alias {
+            if prev_alias.map(is_protected_alias).unwrap_or(false)
+                || options
+                    .get_alias()
+                    .map(|alias| self.has_stream(alias))
+                    .unwrap_or(false)
+            {
+                // user_input, user_output and user_error cannot be realiased,
+                // and realiasing cannot shadow an existing stream.
+                options.set_alias_to_atom_opt(prev_alias);
+                return;
+            }
+
             if let Some(prev_alias) = prev_alias {
                 self.stream_aliases.swap_remove(&prev_alias);
             }
@@ -516,6 +530,11 @@ impl IndexStore {
         self.stream_aliases.contains_key(&alias)
     }
 
+    /// ## Warning
+    ///
+    /// The returned stream's options should only be modified through
+    /// [`IndexStore::update_stream_options`], to avoid breaking the
+    /// invariants of [`IndexStore`].
     pub(crate) fn get_stream(&self, alias: Atom) -> Option<Stream> {
         self.stream_aliases.get(&alias).copied()
     }
diff --git a/src/machine/streams.rs b/src/machine/streams.rs
index ca348a0e..b1ecba03 100644
--- a/src/machine/streams.rs
+++ b/src/machine/streams.rs
@@ -1476,6 +1476,10 @@ impl MachineState {
         }
     }
 
+    /// ## Warning
+    ///
+    /// The options of streams stored in `Machine::indices` should only
+    /// be modified through [`IndexStore::update_stream_options`].
     pub(crate) fn get_stream_options(
         &mut self,
         alias: HeapCellValue,
@@ -1591,6 +1595,19 @@ impl MachineState {
         options
     }
 
+    /// If `addr` is a [`Cons`](HeapCellValueTag::Cons) to a stream, then returns it.
+    ///
+    /// If it is an atom or a string, then this searches for the corresponding stream
+    /// inside of [`self.indices`], returning it.
+    ///
+    /// ## Warning
+    ///
+    /// **Do not directly modify [`stream.options_mut()`](Stream::options_mut)
+    /// on the returned stream.**
+    ///
+    /// Other functions rely on the invariants of [`IndexStore`], which may
+    /// become invalidated by the direct modification of a stream's option (namely,
+    /// its alias name). Instead, use [`IndexStore::update_stream_options`].
     pub(crate) fn get_stream_or_alias(
         &mut self,
         addr: HeapCellValue,
@@ -1953,14 +1970,40 @@ mod test {
         let mut machine = MachineBuilder::new().build();
 
         let results = machine
-            .run_query(r#"
+            .run_query(
+                r#"
                 \+ \+ (
                     open("README.md", read, S, [alias(readme)]),
                     open(stream(S), read, _, [alias(another_alias)]),
                     close(S)
                 ),
                 open("README.md", read, _, [alias(readme)]).
-            "#)
+            "#,
+            )
+            .collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_ok());
+    }
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn close_realiased_user_output() {
+        let mut machine = MachineBuilder::new()
+            .with_streams(StreamConfig::in_memory())
+            .build();
+
+        let results = machine
+            .run_query(
+                r#"
+                \+ \+ (
+                    open("README.md", read, S),
+                    open(stream(S), read, _, [alias(user_output)]),
+                    close(S)
+                ),
+                write(user_output, hello).
+            "#,
+            )
             .collect::<Vec<_>>();
 
         assert_eq!(results.len(), 1);