Attachment 429309 Details for Bug 693366

[patch] fixes for CommunicationObject

co.20110509.diff (text/plain), 13.16 KB, created by Sebastien Pouliot on 2011-05-12 12:38:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Sebastien Pouliot

Created: 2011-05-12 12:38:19 UTC

Size: 13.16 KB

patch

obsolete

>diff --git a/mcs/class/System.ServiceModel/System.ServiceModel.Channels/CommunicationObject.cs b/mcs/class/System.ServiceModel/System.ServiceModel.Channels/CommunicationObject.cs
>index 88a1864..6f05b2a 100644
>--- a/mcs/class/System.ServiceModel/System.ServiceModel.Channels/CommunicationObject.cs
>+++ b/mcs/class/System.ServiceModel/System.ServiceModel.Channels/CommunicationObject.cs
>@@ -36,8 +36,8 @@ namespace System.ServiceModel.Channels
> 	{
> 		object mutex;
> 		CommunicationState state = CommunicationState.Created;
>-		TimeSpan default_open_timeout = TimeSpan.FromMinutes (1), default_close_timeout = TimeSpan.FromMinutes (1);
> 		bool aborted;
>+		bool closing;
> 
> 		protected CommunicationObject ()
> 			: this (new object ())
>@@ -87,10 +87,24 @@ namespace System.ServiceModel.Channels
> 
> 		public void Abort ()
> 		{
>-			if (State != CommunicationState.Closed) {
>-				OnAbort ();
>-				ProcessClosed ();
>+			if (aborted || (state == CommunicationState.Closed) || (state == CommunicationState.Closing))
>+				return;
>+
>+			// note: an explicit call to Abort is a bit different from an internal abort
>+			aborted = true;
>+
>+			if (!closing) {
>+				try {
>+					ProcessClosing ();
>+				}
>+				catch {
>+					ProcessClosed ();
>+					throw;
>+				}
> 			}
>+
>+			OnAbort ();
>+			ProcessClosed ();
> 		}
> 
> 		protected void Fault ()
>@@ -101,7 +115,7 @@ namespace System.ServiceModel.Channels
> 		public IAsyncResult BeginClose (AsyncCallback callback,
> 			object state)
> 		{
>-			return BeginClose (default_close_timeout, callback, state);
>+			return BeginClose (DefaultCloseTimeout, callback, state);
> 		}
> 
> 		public IAsyncResult BeginClose (TimeSpan timeout,
>@@ -116,7 +130,7 @@ namespace System.ServiceModel.Channels
> 		public IAsyncResult BeginOpen (AsyncCallback callback,
> 			object state)
> 		{
>-			return BeginOpen (default_open_timeout, callback, state);
>+			return BeginOpen (DefaultOpenTimeout, callback, state);
> 		}
> 
> 		public IAsyncResult BeginOpen (TimeSpan timeout,
>@@ -128,18 +142,28 @@ namespace System.ServiceModel.Channels
> 
> 		public void Close ()
> 		{
>-			Close (default_close_timeout);
>+			Close (DefaultCloseTimeout);
> 		}
> 
> 		public void Close (TimeSpan timeout)
> 		{
>-			if (State == CommunicationState.Created)
>-				Abort ();
>-			else {
>-				ProcessClosing ();
>+			CommunicationState s = State;
>+			if ((s == CommunicationState.Closed) || (s == CommunicationState.Closing))
>+				return;
>+
>+			bool fault = s == CommunicationState.Faulted;
>+
>+			ProcessClosing ();
>+
>+			if (fault || s == CommunicationState.Created)
>+				OnAbort ();
>+			else
> 				OnClose (timeout);
>-				ProcessClosed ();
>-			}
>+
>+			ProcessClosed ();
>+
>+			if (fault)
>+				throw new CommunicationObjectFaultedException ();
> 		}
> 
> 		public void EndClose (IAsyncResult result)
>@@ -161,7 +185,7 @@ namespace System.ServiceModel.Channels
> 
> 		public void Open ()
> 		{
>-			Open (default_open_timeout);
>+			Open (DefaultOpenTimeout);
> 		}
> 
> 		public void Open (TimeSpan timeout)
>@@ -184,9 +208,9 @@ namespace System.ServiceModel.Channels
> 		void ProcessClosing ()
> 		{
> 			lock (ThisLock) {
>-				if (State == CommunicationState.Faulted)
>-					throw new CommunicationObjectFaultedException ();
>-				OnClosing ();
>+				closing = true;
>+				if (state != CommunicationState.Closing)
>+					OnClosing ();
> 				if (state != CommunicationState.Closing) {
> 					state = CommunicationState.Faulted;
> 					throw new InvalidOperationException (String.Format ("Communication object {0} has an overriden OnClosing method that does not call base OnClosing method (declared in {1} type).", this.GetType (), GetType ().GetMethod ("OnClosing", BindingFlags.NonPublic | BindingFlags.Instance).DeclaringType));
>@@ -199,14 +223,16 @@ namespace System.ServiceModel.Channels
> 			state = CommunicationState.Closing;
> 			// This means, if this method is overriden, then
> 			// Opening event is surpressed.
>-			if (Closing != null)
>-				Closing (this, new EventArgs ());
>+			EventHandler handler = Closing;
>+			if (handler != null)
>+				handler (this, EventArgs.Empty);
> 		}
> 
> 		void ProcessClosed ()
> 		{
> 			lock (ThisLock) {
>-				OnClosed ();
>+				if (state != CommunicationState.Closed)
>+					OnClosed ();
> 				if (state != CommunicationState.Closed) {
> 					state = CommunicationState.Faulted;
> 					throw new InvalidOperationException (String.Format ("Communication object {0} has an overriden OnClosed method that does not call base OnClosed method (declared in {1} type).", this.GetType (), GetType ().GetMethod ("OnClosed", BindingFlags.NonPublic | BindingFlags.Instance).DeclaringType));
>@@ -219,8 +245,9 @@ namespace System.ServiceModel.Channels
> 			state = CommunicationState.Closed;
> 			// This means, if this method is overriden, then
> 			// Closed event is surpressed.
>-			if (Closed != null)
>-				Closed (this, new EventArgs ());
>+			EventHandler handler = Closed;
>+			if (handler != null)
>+				handler (this, EventArgs.Empty);
> 		}
> 
> 		protected abstract void OnEndClose (IAsyncResult result);
>@@ -230,8 +257,12 @@ namespace System.ServiceModel.Channels
> 		void ProcessFaulted ()
> 		{
> 			lock (ThisLock) {
>-				if (State == CommunicationState.Faulted)
>-					throw new CommunicationObjectFaultedException ();
>+				// as documented: Fault does nothing if state is already Faulted or when Closed
>+				if (state == CommunicationState.Faulted || state == CommunicationState.Closed)
>+					return;
>+
>+				// note: Faulted is already set when OnFaulted is called
>+				state = CommunicationState.Faulted;
> 				OnFaulted ();
> 				if (state != CommunicationState.Faulted) {
> 					state = CommunicationState.Faulted; // FIXME: am not sure if this makes sense ...
>@@ -242,11 +273,11 @@ namespace System.ServiceModel.Channels
> 
> 		protected virtual void OnFaulted ()
> 		{
>-			state = CommunicationState.Faulted;
> 			// This means, if this method is overriden, then
> 			// Faulted event is surpressed.
>-			if (Faulted != null)
>-				Faulted (this, new EventArgs ());
>+			EventHandler handler = Faulted;
>+			if (handler != null)
>+				handler (this, EventArgs.Empty);
> 		}
> 
> 		protected abstract void OnOpen (TimeSpan timeout);
>@@ -265,8 +296,9 @@ namespace System.ServiceModel.Channels
> 		protected virtual void OnOpened ()
> 		{
> 			state = CommunicationState.Opened;
>-			if (Opened != null)
>-				Opened (this, new EventArgs ());
>+			EventHandler handler = Opened;
>+			if (handler != null)
>+				handler (this, EventArgs.Empty);
> 		}
> 
> 		void ProcessOpening ()
>@@ -286,21 +318,25 @@ namespace System.ServiceModel.Channels
> 			state = CommunicationState.Opening;
> 			// This means, if this method is overriden, then
> 			// Opening event is surpressed.
>-			if (Opening != null)
>-				Opening (this, new EventArgs ());
>+			EventHandler handler = Opening;
>+			if (handler != null)
>+				handler (this, EventArgs.Empty);
> 		}
> 
> 		protected void ThrowIfDisposed ()
> 		{
>-			if (IsDisposed)
>-				throw new ObjectDisposedException (String.Format ("This communication object {0} is already disposed.", GetCommunicationObjectType ()));
>+			if (aborted)
>+				throw new CommunicationObjectAbortedException ();
>+			if (state == CommunicationState.Faulted)
>+				throw new CommunicationObjectFaultedException ();
>+			// IsDisposed can be false and still we need to throw ObjectDisposedException
>+			if ((state == CommunicationState.Closing) || (state == CommunicationState.Closed))
>+				throw new ObjectDisposedException (String.Format ("This communication object {0} is already disposed (state {1}).", GetCommunicationObjectType (), State));
> 		}
> 
> 		protected void ThrowIfDisposedOrNotOpen ()
> 		{
> 			ThrowIfDisposed ();
>-			if (State == CommunicationState.Faulted)
>-				throw new CommunicationObjectFaultedException ();
> 			if (State != CommunicationState.Opened)
> 				throw new InvalidOperationException (String.Format ("The communication object {0} must be at opened state.", GetCommunicationObjectType ()));
> 		}
>@@ -308,14 +344,8 @@ namespace System.ServiceModel.Channels
> 		protected void ThrowIfDisposedOrImmutable ()
> 		{
> 			ThrowIfDisposed ();
>-			// hmm, according to msdn, Closing is OK here.
>-			switch (State) {
>-			case CommunicationState.Faulted:
>-				throw new CommunicationObjectFaultedException ();
>-			case CommunicationState.Opening:
>-			case CommunicationState.Opened:
>+			if ((state == CommunicationState.Opening) || (state == CommunicationState.Opened))
> 				throw new InvalidOperationException (String.Format ("The communication object {0} is not at created state but at {1} state.", GetType (), State));
>-			}
> 		}
> 
> 		protected virtual Type GetCommunicationObjectType ()
>diff --git a/mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/CommunicationObjectSyncTest.cs b/mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/CommunicationObjectSyncTest.cs
>index 1650bea..4789b29 100644
>--- a/mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/CommunicationObjectSyncTest.cs
>+++ b/mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/CommunicationObjectSyncTest.cs
>@@ -168,11 +168,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Abort ()
> 		{
> 			int closing = 0;
>@@ -237,11 +232,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Close ()
> 		{
> 			int opening = 0;
>@@ -340,11 +330,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Close_Fault ()
> 		{
> 			bool faulted = false;
>@@ -361,11 +346,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Open_Close_Abort ()
> 		{
> 			int opening = 0;
>@@ -453,11 +433,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Fault_Abort ()
> 		{
> 			int opening = 0;
>@@ -546,11 +521,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Fault_Open_Close ()
> 		{
> 			int opening = 0;
>@@ -641,11 +611,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Fault_Open_Abort_Close ()
> 		{
> 			int opening = 0;
>@@ -733,11 +698,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void Create_Open_Open ()
> 		{
> 			int opening = 0;
>@@ -777,11 +737,6 @@ namespace MoonTest.ServiceModel {
> 		// ThrowIfDisposed throws an exception if the state is Closing, Closed or Faulted.
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposed_Open_Close ()
> 		{
> 			bool opening = false;
>@@ -843,11 +798,6 @@ namespace MoonTest.ServiceModel {
> 
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposed_Fault_Abort ()
> 		{
> 			bool opening = false;
>@@ -910,11 +860,6 @@ namespace MoonTest.ServiceModel {
> 		// ThrowIfDisposedOrImmutable throws an exception if the state is not Created.
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposedOrImmutable_Open_Close ()
> 		{
> 			bool opening = false;
>@@ -979,11 +924,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposedOrImmutable_Fault_Abort ()
> 		{
> 			bool opening = false;
>@@ -1050,11 +990,6 @@ namespace MoonTest.ServiceModel {
> 		// ThrowIfDisposedOrNotOpen throws an exception if the state is not Opened. 
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposedOrNotOpen_Open_Close ()
> 		{
> 			bool opening = false;
>@@ -1119,11 +1054,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void ThrowIfDisposedOrNotOpen_Fault_Abort ()
> 		{
> 			bool opening = false;
>@@ -1196,11 +1126,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void NoOnFault ()
> 		{
> 			bool faulted = false;
>@@ -1223,11 +1148,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void OnFaultThrowing ()
> 		{
> 			FaultCommunicationObject co = new FaultCommunicationObject ();
>@@ -1239,11 +1159,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void AbortWhileAborting ()
> 		{
> 			int closing = 0;
>@@ -1267,11 +1182,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void AbortWhileClosing ()
> 		{
> 			int closing = 0;
>@@ -1303,11 +1213,6 @@ namespace MoonTest.ServiceModel {
> 		}
> 
> 		[TestMethod]
>-#if NET_2_1
>-		[MoonlightBug]
>-#else
>-		[NUnit.Framework.Ignore]
>-#endif
> 		public void NoOnAbort ()
> 		{
> 			bool closing = false;

Actions: View | Diff

Attachments on bug 693366: 429309