From 91f6898b26caa8f74533dbd90478cf4103251abe Mon Sep 17 00:00:00 2001 From: Melanie Date: Thu, 6 Aug 2009 22:03:20 +0100 Subject: [PATCH] |From: James J Greensky |Date: Wed, 5 Aug 2009 09:51:52 -0700 |Subject: [PATCH] Closed two major memory leaks for scripted objects | |Two major memory leaks for the scripted objects were fixed |- One leak had to do with remoting acrossing app domains. When a script and | its controlling agent communicate across an application boundary, it calls | functions on a stub proxy object that then invokes the remote method on | the object in the other app domain. These stub objects (two for each script) | were setup to have infinate lifetimes and were never being garbage collected. |- The second leak was the result of adding a scene object part instance method | to a scene event and never removing it. This cause the event's delegate list | to maintain a link to that object which is then never freed as the scene event | object is never destroyed. Patch applied, please direct feedback to me. Possible issue: Longtime idle scripts like vendors may fail. --- .../Region/Framework/Scenes/EventManager.cs | 11 ++++---- .../Framework/Scenes/SceneObjectPart.cs | 16 ++++++------ .../Region/ScriptEngine/Interfaces/IScript.cs | 2 ++ .../Shared/Api/Implementation/LSL_Api.cs | 5 ++-- .../Shared/Api/Implementation/OSSL_Api.cs | 7 +++--- .../Shared/Api/Runtime/Executor.cs | 22 +--------------- .../Shared/Api/Runtime/LSL_Constants.cs | 2 +- .../Shared/Api/Runtime/ScriptBase.cs | 25 ++++++++++++------- .../Shared/Api/Runtime/ScriptSponsor.cs | 12 ++++++--- .../Shared/Instance/ScriptInstance.cs | 25 +++++++++++++------ .../Region/ScriptEngine/XEngine/XEngine.cs | 14 +++++------ 11 files changed, 73 insertions(+), 68 deletions(-) diff --git a/OpenSim/Region/Framework/Scenes/EventManager.cs b/OpenSim/Region/Framework/Scenes/EventManager.cs index 086496ed80..1d4d6d7708 100644 --- a/OpenSim/Region/Framework/Scenes/EventManager.cs +++ b/OpenSim/Region/Framework/Scenes/EventManager.cs @@ -953,11 +953,12 @@ namespace OpenSim.Region.Framework.Scenes // this lets us keep track of nasty script events like timer, etc. public void TriggerTimerEvent(uint objLocalID, double Interval) { - handlerScriptTimerEvent = OnScriptTimerEvent; - if (handlerScriptTimerEvent != null) - { - handlerScriptTimerEvent(objLocalID, Interval); - } + throw new NotImplementedException("TriggerTimerEvent was thought to be not used anymore and the registration for the event from scene object part has been commented out due to a memory leak"); + //handlerScriptTimerEvent = OnScriptTimerEvent; + //if (handlerScriptTimerEvent != null) + //{ + // handlerScriptTimerEvent(objLocalID, Interval); + //} } /// diff --git a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs index bc117090f4..61dfa527f2 100644 --- a/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs +++ b/OpenSim/Region/Framework/Scenes/SceneObjectPart.cs @@ -3673,14 +3673,14 @@ if (m_shape != null) { return; } - if ((GetEffectiveObjectFlags() & (uint)PrimFlags.Scripted) != 0) - { - m_parentGroup.Scene.EventManager.OnScriptTimerEvent += handleTimerAccounting; - } - else - { - m_parentGroup.Scene.EventManager.OnScriptTimerEvent -= handleTimerAccounting; - } + //if ((GetEffectiveObjectFlags() & (uint)PrimFlags.Scripted) != 0) + //{ + // m_parentGroup.Scene.EventManager.OnScriptTimerEvent += handleTimerAccounting; + //} + //else + //{ + // m_parentGroup.Scene.EventManager.OnScriptTimerEvent -= handleTimerAccounting; + //} LocalFlags=(PrimFlags)objectflagupdate; diff --git a/OpenSim/Region/ScriptEngine/Interfaces/IScript.cs b/OpenSim/Region/ScriptEngine/Interfaces/IScript.cs index b9618d10f5..726dabce66 100644 --- a/OpenSim/Region/ScriptEngine/Interfaces/IScript.cs +++ b/OpenSim/Region/ScriptEngine/Interfaces/IScript.cs @@ -41,5 +41,7 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase Dictionary GetVars(); void SetVars(Dictionary vars); void ResetVars(); + + void Close(); } } diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs index bc36fda53c..5f9b09b680 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/LSL_Api.cs @@ -119,14 +119,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api AsyncCommands = new AsyncCommandManager(ScriptEngine); } - // Object never expires public override Object InitializeLifetimeService() { ILease lease = (ILease)base.InitializeLifetimeService(); if (lease.CurrentState == LeaseState.Initial) { - lease.InitialLeaseTime = TimeSpan.Zero; + lease.InitialLeaseTime = TimeSpan.FromMinutes(1.0); + lease.RenewOnCallTime = TimeSpan.FromSeconds(10.0); + lease.SponsorshipTimeout = TimeSpan.FromMinutes(1.0); } return lease; } diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/OSSL_Api.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/OSSL_Api.cs index 7c878b85aa..b447cfb032 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/OSSL_Api.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Implementation/OSSL_Api.cs @@ -159,16 +159,15 @@ namespace OpenSim.Region.ScriptEngine.Shared.Api } } - // - // Never expire this object - // public override Object InitializeLifetimeService() { ILease lease = (ILease)base.InitializeLifetimeService(); if (lease.CurrentState == LeaseState.Initial) { - lease.InitialLeaseTime = TimeSpan.Zero; + lease.InitialLeaseTime = TimeSpan.FromMinutes(1.0); + lease.RenewOnCallTime = TimeSpan.FromSeconds(10.0); + lease.SponsorshipTimeout = TimeSpan.FromMinutes(1.0); } return lease; } diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/Executor.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/Executor.cs index 2501752039..7f67599c92 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/Executor.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/Executor.cs @@ -35,7 +35,7 @@ using log4net; namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase { - public class Executor : MarshalByRefObject + public class Executor { // private static readonly ILog m_log = LogManager.GetLogger(MethodBase.GetCurrentMethod().DeclaringType); @@ -89,26 +89,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase initEventFlags(); } - /// - /// Make sure our object does not timeout when in AppDomain. (Called by ILease base class) - /// - /// - public override Object InitializeLifetimeService() - { - //m_log.Debug("Executor: InitializeLifetimeService()"); - // return null; - ILease lease = (ILease)base.InitializeLifetimeService(); - - if (lease.CurrentState == LeaseState.Initial) - { - lease.InitialLeaseTime = TimeSpan.Zero; // TimeSpan.FromMinutes(1); - // lease.SponsorshipTimeout = TimeSpan.FromMinutes(2); - // lease.RenewOnCallTime = TimeSpan.FromSeconds(2); - } - return lease; - } - - public scriptEvents GetStateEventFlags(string state) { //m_log.Debug("Get event flags for " + state); diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/LSL_Constants.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/LSL_Constants.cs index 9068634c87..753ca55d08 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/LSL_Constants.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/LSL_Constants.cs @@ -32,7 +32,7 @@ using LSLInteger = OpenSim.Region.ScriptEngine.Shared.LSL_Types.LSLInteger; namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase { - public partial class ScriptBaseClass : MarshalByRefObject + public partial class ScriptBaseClass { // LSL CONSTANTS public static readonly LSLInteger TRUE = new LSLInteger(1); diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptBase.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptBase.cs index 964fe4ca29..d119a7739d 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptBase.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptBase.cs @@ -26,6 +26,7 @@ */ using System; +using System.Runtime.Remoting; using System.Runtime.Remoting.Lifetime; using System.Security.Permissions; using System.Threading; @@ -34,26 +35,23 @@ using System.Collections; using System.Collections.Generic; using OpenSim.Region.ScriptEngine.Interfaces; using OpenSim.Region.ScriptEngine.Shared; +using OpenSim.Region.ScriptEngine.Shared.Api.Runtime; namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase { public partial class ScriptBaseClass : MarshalByRefObject, IScript { private Dictionary inits = new Dictionary(); + private ScriptSponsor m_sponser; - // Object expires if we don't keep it alive - // sponsor will be added on object load - [SecurityPermissionAttribute(SecurityAction.Demand, - Flags = SecurityPermissionFlag.Infrastructure)] public override Object InitializeLifetimeService() { ILease lease = (ILease)base.InitializeLifetimeService(); if (lease.CurrentState == LeaseState.Initial) { - lease.InitialLeaseTime = TimeSpan.Zero; -// lease.InitialLeaseTime = TimeSpan.FromMinutes(1); -// lease.SponsorshipTimeout = TimeSpan.FromMinutes(2); -// lease.RenewOnCallTime = TimeSpan.FromSeconds(2); + lease.InitialLeaseTime = TimeSpan.FromMinutes(1.0); + lease.RenewOnCallTime = TimeSpan.FromSeconds(10.0); + lease.SponsorshipTimeout = TimeSpan.FromMinutes(1.0); } return lease; } @@ -66,7 +64,6 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase } #endif - public ScriptBaseClass() { m_Executor = new Executor(this); @@ -81,6 +78,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase inits[type] = mi; } } + + m_sponser = new ScriptSponsor(); } private Executor m_Executor = null; @@ -112,6 +111,9 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase if (!inits.ContainsKey(api)) return; + ILease lease = (ILease)RemotingServices.GetLifetimeService(data as MarshalByRefObject); + lease.Register(m_sponser); + MethodInfo mi = inits[api]; Object[] args = new Object[1]; @@ -122,6 +124,11 @@ namespace OpenSim.Region.ScriptEngine.Shared.ScriptBase m_InitialValues = GetVars(); } + public void Close() + { + m_sponser.Close(); + } + public Dictionary GetVars() { Dictionary vars = new Dictionary(); diff --git a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptSponsor.cs b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptSponsor.cs index a2da14e013..977ac3074a 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptSponsor.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Api/Runtime/ScriptSponsor.cs @@ -32,15 +32,19 @@ using System.Text; namespace OpenSim.Region.ScriptEngine.Shared.Api.Runtime { - [Serializable] public class ScriptSponsor : MarshalByRefObject, ISponsor { - // In theory: I execute, therefore I am. - // If GC collects this class then sponsorship will expire + private bool m_closed = false; + public TimeSpan Renewal(ILease lease) { - return TimeSpan.FromMinutes(2); + if (!m_closed) + return lease.InitialLeaseTime; + return TimeSpan.FromTicks(0); } + + public void Close() { m_closed = true; } + #if DEBUG // For tracing GC while debugging public static bool GCDummy = false; diff --git a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs index 8168300c55..6d62249ae1 100644 --- a/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs +++ b/OpenSim/Region/ScriptEngine/Shared/Instance/ScriptInstance.cs @@ -96,7 +96,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance private string m_CurrentState = String.Empty; private UUID m_RegionID = UUID.Zero; - //private ISponsor m_ScriptSponsor; + private ScriptSponsor m_ScriptSponsor; + private bool m_destroyed = false; private Dictionary, KeyValuePair> m_LineMap; @@ -261,12 +262,9 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance Path.GetFileNameWithoutExtension(assembly), "SecondLife.Script"); - // Add a sponsor to the script -// ISponsor scriptSponsor = new ScriptSponsor(); -// ILease lease = (ILease)RemotingServices.GetLifetimeService(m_Script as MarshalByRefObject); -// lease.Register(scriptSponsor); - //m_ScriptSponsor = scriptSponsor; - + m_ScriptSponsor = new ScriptSponsor(); + ILease lease = (ILease)RemotingServices.GetLifetimeService(m_Script as ScriptBaseClass); + lease.Register(m_ScriptSponsor); } catch (Exception) { @@ -449,6 +447,13 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance { ReleaseControls(); AsyncCommandManager.RemoveScript(m_Engine, m_LocalID, m_ItemID); + + m_Script.Close(); + m_ScriptSponsor.Close(); + ILease lease = (ILease)RemotingServices.GetLifetimeService(m_Script as ScriptBaseClass); + lease.Unregister(m_ScriptSponsor); + + m_destroyed = true; } public void RemoveState() @@ -884,6 +889,8 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance public void SaveState(string assembly) { + + // If we're currently in an event, just tell it to save upon return // if (m_InEvent) @@ -892,6 +899,10 @@ namespace OpenSim.Region.ScriptEngine.Shared.Instance return; } + // Data may not be available as the script has already been destroyed + if (m_destroyed == true) + return; + PluginData = AsyncCommandManager.GetSerializationData(m_Engine, m_ItemID); string xml = ScriptSerializer.Serialize(this); diff --git a/OpenSim/Region/ScriptEngine/XEngine/XEngine.cs b/OpenSim/Region/ScriptEngine/XEngine/XEngine.cs index c7673c7a75..ff75e2ff4f 100644 --- a/OpenSim/Region/ScriptEngine/XEngine/XEngine.cs +++ b/OpenSim/Region/ScriptEngine/XEngine/XEngine.cs @@ -272,6 +272,10 @@ namespace OpenSim.Region.ScriptEngine.XEngine instance.ClearQueue(); instance.Stop(0); + // Release events, timer, etc + // + instance.DestroyScriptInstance(); + // Unload scripts and app domains // Must be done explicitly because they have infinite // lifetime @@ -282,10 +286,6 @@ namespace OpenSim.Region.ScriptEngine.XEngine m_DomainScripts.Remove(instance.AppDomain); UnloadAppDomain(instance.AppDomain); } - - // Release events, timer, etc - // - instance.DestroyScriptInstance(); } m_Scripts.Clear(); m_PrimObjects.Clear(); @@ -802,6 +802,9 @@ namespace OpenSim.Region.ScriptEngine.XEngine } } + instance.RemoveState(); + instance.DestroyScriptInstance(); + m_DomainScripts[instance.AppDomain].Remove(instance.ItemID); if (m_DomainScripts[instance.AppDomain].Count == 0) { @@ -809,9 +812,6 @@ namespace OpenSim.Region.ScriptEngine.XEngine UnloadAppDomain(instance.AppDomain); } - instance.RemoveState(); - instance.DestroyScriptInstance(); - instance = null; ObjectRemoved handlerObjectRemoved = OnObjectRemoved;