Fix issue where lsl -> c# generation in co-operative termination mode did not correctly handle single statement versions of for, while and do-while loops.

Add regression tests to validate the fix.
This problem will not affect the default abort termination mode.

2 files changed, 171 insertions, 22 deletions

diff --git a/OpenSim/Region/ScriptEngine/Shared/CodeTools/CSCodeGenerator.cs b/OpenSim/Region/ScriptEngine/Shared/CodeTools/CSCodeGenerator.cs
index 985e598..9e32f40 100644
--- a/OpenSim/Region/ScriptEngine/Shared/CodeTools/CSCodeGenerator.cs
+++ b/OpenSim/Region/ScriptEngine/Shared/CodeTools/CSCodeGenerator.cs
@@ -31,7 +31,6 @@ using System.Collections.Generic;
 using System.Reflection;
 using log4net;
 using Tools;
-
 using OpenSim.Region.Framework.Interfaces;
 
 namespace OpenSim.Region.ScriptEngine.Shared.CodeTools
@@ -479,20 +478,27 @@ namespace OpenSim.Region.ScriptEngine.Shared.CodeTools
         {
             string retstr = String.Empty;
             bool printSemicolon = true;
-
-            retstr += Indent();
+            bool transformToBlock = false;
 
             if (m_insertCoopTerminationChecks)
             {
-                // We have to check in event functions as well because the user can manually call these.
-                if (previousSymbol is GlobalFunctionDefinition 
-                    || previousSymbol is WhileStatement 
+                // A non-braced single line do while structure cannot contain multiple statements.
+                // So to insert the termination check we change this to a braced control structure instead.
+                if (previousSymbol is WhileStatement 
                     || previousSymbol is DoWhileStatement 
-                    || previousSymbol is ForLoop
-                    || previousSymbol is StateEvent)
-                retstr += Generate(m_coopTerminationCheck);
+                    || previousSymbol is ForLoop)
+                {
+                    transformToBlock = true;
+
+                    // FIXME: This will be wrongly indented because the previous for/while/dowhile will have already indented.
+                    retstr += GenerateIndentedLine("{");
+
+                    retstr += GenerateIndentedLine(m_coopTerminationCheck);
+                }
             }
 
+            retstr += Indent();
+
             if (0 < s.kids.Count)
             {
                 // Jump label prints its own colon, we don't need a semicolon.
@@ -508,6 +514,12 @@ namespace OpenSim.Region.ScriptEngine.Shared.CodeTools
             if (printSemicolon)
                 retstr += GenerateLine(";");
 
+            if (transformToBlock)
+            {
+                // FIXME: This will be wrongly indented because the for/while/dowhile is currently handling the unindent
+                retstr += GenerateIndentedLine("}");
+            }
+
             return retstr;
         }
 
diff --git a/OpenSim/Region/ScriptEngine/Shared/Instance/Tests/CoopTerminationTests.cs b/OpenSim/Region/ScriptEngine/Shared/Instance/Tests/CoopTerminationTests.cs
index 2c80826..7ea30bf1 100644
--- a/OpenSim/Region/ScriptEngine/Shared/Instance/Tests/CoopTerminationTests.cs
+++ b/OpenSim/Region/ScriptEngine/Shared/Instance/Tests/CoopTerminationTests.cs
@@ -55,10 +55,22 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
 
         private OSChatMessage m_osChatMessageReceived;
 
+        /// <summary>
+        /// Number of chat messages received so far.  Reset before each test.
+        /// </summary>
+        private int m_chatMessagesReceived;
+
+        /// <summary>
+        /// Number of chat messages expected.  m_chatEvent is not fired until this number is reached or exceeded.
+        /// </summary>
+        private int m_chatMessagesThreshold;
+
         [SetUp]
         public void Init()
         {
             m_osChatMessageReceived = null;
+            m_chatMessagesReceived = 0;
+            m_chatMessagesThreshold = 0;
             m_chatEvent = new AutoResetEvent(false);
             m_stoppedEvent = new AutoResetEvent(false);
 
@@ -126,6 +138,25 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
         }
 
         [Test]
+        public void TestNoStopOnSingleStatementForLoop()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            string script = 
+@"default
+{    
+    state_entry()
+    {
+        integer i = 0;        
+        for (i = 0; i <= 1; i++) llSay(0, ""Iter "" + (string)i);
+    }
+}";
+
+            TestSingleStatementNoStop(script);
+        }
+
+        [Test]
         public void TestStopOnLongSingleStatementForLoop()
         {
             TestHelpers.InMethod();
@@ -139,8 +170,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
         integer i = 0;
         llSay(0, ""Thin Lizzy"");
         
-        for (i = 0; i < 2147483647; i++)        
-            llSay(0, ""Iter "" + (string)i);
+        for (i = 0; i < 2147483647; i++) llSay(0, ""Iter "" + (string)i);
     }
 }";
 
@@ -172,6 +202,25 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
         }
 
         [Test]
+        public void TestNoStopOnSingleStatementWhileLoop()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            string script = 
+@"default
+{    
+    state_entry()
+    {
+        integer i = 0;        
+        while (i < 2) llSay(0, ""Iter "" + (string)i++);
+    }
+}";
+
+            TestSingleStatementNoStop(script);
+        }
+
+        [Test]
         public void TestStopOnLongSingleStatementWhileLoop()
         {
             TestHelpers.InMethod();
@@ -218,7 +267,50 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
         }
 
         [Test]
-        public void TestStopOnLongDoWhileLoop()
+        public void TestNoStopOnSingleStatementDoWhileLoop()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            string script = 
+@"default
+{    
+    state_entry()
+    {
+        integer i = 0;
+
+        do llSay(0, ""Iter "" + (string)i++);
+        while (i < 2);
+    }
+}";
+
+            TestSingleStatementNoStop(script);
+        }
+
+        [Test]
+        public void TestStopOnLongSingleStatementDoWhileLoop()
+        {
+            TestHelpers.InMethod();
+//            TestHelpers.EnableLogging();
+
+            string script = 
+@"default
+{    
+    state_entry()
+    {
+        integer i = 0;
+        llSay(0, ""Thin Lizzy"");
+
+        do llSay(0, ""Iter "" + (string)i++);
+        while (1 == 1);
+    }
+}";
+
+            TestStop(script);
+        }
+
+        [Test]
+        public void TestStopOnLongCompoundStatementDoWhileLoop()
         {
             TestHelpers.InMethod();
 //            TestHelpers.EnableLogging();
@@ -234,7 +326,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance.Tests
         do 
         {
             llSay(0, ""Iter "" + (string)i++);
-} while (1 == 1);
+        } while (1 == 1);
     }
 }";
 
@@ -320,14 +412,13 @@ default
             TestStop(script);
         }
 
-        private void TestStop(string script)
+        private SceneObjectPart CreateScript(string script, string itemName, UUID userId)
         {
-            UUID userId = TestHelpers.ParseTail(0x1);
 //            UUID objectId = TestHelpers.ParseTail(0x100);
 //            UUID itemId = TestHelpers.ParseTail(0x3);
-            string itemName = "TestStop() Item";
 
-            SceneObjectGroup so = SceneHelpers.CreateSceneObject(1, userId, "TestStop", 0x100);
+            SceneObjectGroup so 
+                = SceneHelpers.CreateSceneObject(1, userId, string.Format("Object for {0}", itemName), 0x100);
             m_scene.AddNewSceneObject(so, true);
 
             InventoryItemBase itemTemplate = new InventoryItemBase();
@@ -338,14 +429,57 @@ default
 
             m_scene.EventManager.OnChatFromWorld += OnChatFromWorld;
 
-            SceneObjectPart partWhereRezzed = m_scene.RezNewScript(userId, itemTemplate, script);
+            return m_scene.RezNewScript(userId, itemTemplate, script);
+        }
+
+        private void TestSingleStatementNoStop(string script)
+        {
+            // In these tests we expect to see at least 2 chat messages to confirm that the loop is working properly.
+            m_chatMessagesThreshold = 2;
 
+            UUID userId = TestHelpers.ParseTail(0x1);
+//            UUID objectId = TestHelpers.ParseTail(0x100);
+//            UUID itemId = TestHelpers.ParseTail(0x3);
+            string itemName = "TestNoStop";
+
+            SceneObjectPart partWhereRezzed = CreateScript(script, itemName, userId);
             TaskInventoryItem rezzedItem = partWhereRezzed.Inventory.GetInventoryItem(itemName);
 
             // Wait for the script to start the event before we try stopping it.
             m_chatEvent.WaitOne(60000);
 
-            Console.WriteLine("Script started with message [{0}]", m_osChatMessageReceived.Message);
+            if (m_osChatMessageReceived == null)
+                Assert.Fail("Script did not start");
+            else
+                Assert.That(m_chatMessagesReceived, Is.EqualTo(2));
+
+            bool running;
+            TaskInventoryItem scriptItem = partWhereRezzed.Inventory.GetInventoryItem(itemName);
+            Assert.That(
+                SceneObjectPartInventory.TryGetScriptInstanceRunning(m_scene, scriptItem, out running), Is.True);
+            Assert.That(running, Is.True);
+        }
+
+        private void TestStop(string script)
+        {
+            // In these tests we're only interested in the first message to confirm that the script has started.
+            m_chatMessagesThreshold = 1;
+
+            UUID userId = TestHelpers.ParseTail(0x1);
+//            UUID objectId = TestHelpers.ParseTail(0x100);
+//            UUID itemId = TestHelpers.ParseTail(0x3);
+            string itemName = "TestStop";
+
+            SceneObjectPart partWhereRezzed = CreateScript(script, itemName, userId);
+            TaskInventoryItem rezzedItem = partWhereRezzed.Inventory.GetInventoryItem(itemName);
+
+            // Wait for the script to start the event before we try stopping it.
+            m_chatEvent.WaitOne(60000);
+
+            if (m_osChatMessageReceived != null)
+                Console.WriteLine("Script started with message [{0}]", m_osChatMessageReceived.Message);
+            else
+                Assert.Fail("Script did not start");
 
             // FIXME: This is a very poor way of trying to avoid a low-probability race condition where the script
             // executes llSay() but has not started the next statement before we try to stop it.
@@ -367,11 +501,14 @@ default
 
         private void OnChatFromWorld(object sender, OSChatMessage oscm)
         {
-            m_scene.EventManager.OnChatFromWorld -= OnChatFromWorld;
             Console.WriteLine("Got chat [{0}]", oscm.Message);
-
             m_osChatMessageReceived = oscm;
-            m_chatEvent.Set();
+
+            if (++m_chatMessagesReceived >= m_chatMessagesThreshold)
+            {
+                m_scene.EventManager.OnChatFromWorld -= OnChatFromWorld;            
+                m_chatEvent.Set();
+            }
         }
     }
 }
\ No newline at end of file