2009-06-08 Atsushi Enomoto <[email protected]>

	* CommunicationObject.cs : OnClosed() requires call to base method.
	  Close() could rather abort the channel, and Abort() does not
	  always invke OnAbort().
	* ChannelBase.cs : add call to base as above.

	* ChannelDispatcher.cs :
	  Add call to base (see CommunicationObject change).

	* InstanceContext.cs :
	  Add call to base (see CommunicationObject change).

	* CommunicationObjectTest.cs : added a couple of close/abort tests
	  for state transition and base call check.


svn path=/trunk/mcs/; revision=135649

mcs/class/System.ServiceModel/System.ServiceModel.Channels/ChangeLog

@@ -1,3 +1,10 @@
+2009-06-08  Atsushi Enomoto  <[email protected]>
+
+	* CommunicationObject.cs : OnClosed() requires call to base method.
+	  Close() could rather abort the channel, and Abort() does not
+	  always invke OnAbort().
+	* ChannelBase.cs : add call to base as above.
+
 2009-06-05  Sebastien Pouliot  <[email protected]>
 
 	* ClientAccessPolicy.cs: Removed

mcs/class/System.ServiceModel/System.ServiceModel.Channels/ChannelBase.cs

@@ -71,6 +71,7 @@ namespace System.ServiceModel.Channels
 
 		protected override void OnClosed ()
 		{
+			base.OnClosed ();
 		}
 
 		TimeSpan IDefaultCommunicationTimeouts.CloseTimeout {

mcs/class/System.ServiceModel/System.ServiceModel.Channels/CommunicationObject.cs

@@ -36,7 +36,7 @@ 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 aborted, on_closed_called;
 
 		protected CommunicationObject ()
 			: this (new object ())
@@ -84,10 +84,12 @@ namespace System.ServiceModel.Channels
 
 		#region Methods
 
-		[MonoTODO]
 		public void Abort ()
 		{
-			OnAbort ();
+			if (State != CommunicationState.Closed) {
+				OnAbort ();
+				ProcessClosed ();
+			}
 		}
 
 		[MonoTODO]
@@ -106,6 +108,8 @@ namespace System.ServiceModel.Channels
 		public IAsyncResult BeginClose (TimeSpan timeout,
 			AsyncCallback callback, object state)
 		{
+			if (State == CommunicationState.Created)
+				return new EventHandler (delegate { Abort (); }).BeginInvoke (null, null, callback, state);
 			ProcessClosing ();
 			return OnBeginClose (timeout, callback, state);
 		}
@@ -130,15 +134,24 @@ namespace System.ServiceModel.Channels
 
 		public void Close (TimeSpan timeout)
 		{
-			ProcessClosing ();
-			OnClose (timeout);
-			ProcessClosed ();
+			if (State == CommunicationState.Created)
+				Abort ();
+			else {
+				ProcessClosing ();
+				OnClose (timeout);
+				ProcessClosed ();
+			}
 		}
 
 		public void EndClose (IAsyncResult result)
 		{
-			OnEndClose (result);
-			ProcessClosed ();
+			if (State == CommunicationState.Created || State == CommunicationState.Closed) {
+				if (!result.IsCompleted)
+					result.AsyncWaitHandle.WaitOne ();
+			} else {
+				OnEndClose (result);
+				ProcessClosed ();
+			}
 		}
 
 		public void EndOpen (IAsyncResult result)
@@ -188,7 +201,10 @@ namespace System.ServiceModel.Channels
 		void ProcessClosed ()
 		{
 			state = CommunicationState.Closed;
+			on_closed_called = false;
 			OnClosed ();
+			if (!on_closed_called)
+				throw new InvalidOperationException ("OnClosed method is implemented but it did not call its base OnClosed method");
 		}
 
 		protected virtual void OnClosed ()
@@ -197,6 +213,7 @@ namespace System.ServiceModel.Channels
 			// Closed event is surpressed.
 			if (Closed != null)
 				Closed (this, new EventArgs ());
+			on_closed_called = true;
 		}
 
 		protected abstract void OnEndClose (IAsyncResult result);

mcs/class/System.ServiceModel/System.ServiceModel.Dispatcher/ChangeLog

@@ -1,3 +1,8 @@
+2009-06-08  Atsushi Enomoto  <[email protected]>
+
+	* ChannelDispatcher.cs :
+	  Add call to base (see CommunicationObject change).
+
 2009-06-01  Atsushi Enomoto  <[email protected]>
 
 	* ChannelDispatcher.cs : handle more errors gracefully.

mcs/class/System.ServiceModel/System.ServiceModel.Dispatcher/ChannelDispatcher.cs

@@ -237,6 +237,7 @@ namespace System.ServiceModel.Dispatcher
 		{
 			if (host != null)
 				host.ChannelDispatchers.Remove (this);
+			base.OnClosed ();
 		}
 
 		protected override void OnEndClose (IAsyncResult result)

mcs/class/System.ServiceModel/System.ServiceModel/ChangeLog

@@ -1,3 +1,8 @@
+2009-06-08  Atsushi Enomoto  <[email protected]>
+
+	* InstanceContext.cs :
+	  Add call to base (see CommunicationObject change).
+
 2009-05-26  Atsushi Enomoto  <[email protected]>
 
 	* OperationContext.cs : IncomingMessage* properties should work

mcs/class/System.ServiceModel/System.ServiceModel/InstanceContext.cs

@@ -169,6 +169,7 @@ namespace System.ServiceModel
 
 		protected override void OnClosed ()
 		{
+			base.OnClosed ();
 		}
 
 		protected override void OnOpened ()

mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/ChangeLog

@@ -1,3 +1,8 @@
+2009-06-08  Atsushi Enomoto  <[email protected]>
+
+	* CommunicationObjectTest.cs : added a couple of close/abort tests
+	  for state transition and base call check.
+
 2009-05-22  Atsushi Enomoto  <[email protected]>
 
 	* MessageEncoderTest.cs : test CreateSessionEncoder().

mcs/class/System.ServiceModel/Test/System.ServiceModel.Channels/CommunicationObjectTest.cs

@@ -34,7 +34,7 @@ namespace MonoTests.System.ServiceModel.Channels
 {
 	class ExtCommObj : CommunicationObject
 	{
-		public new bool Aborted, Opened, Closed;
+		public new bool Aborted, Opened, Closed, OnClosedCalled;
 
 		public ExtCommObj () : base ()
 		{
@@ -98,6 +98,27 @@ namespace MonoTests.System.ServiceModel.Channels
 				throw new Exception ("Already opened");
 			Opened = true;
 		}
+
+		protected override void OnClosed ()
+		{
+			if (OnClosedCalled)
+				throw new Exception ("OnClosed() already called");
+			OnClosedCalled = true;
+			base.OnClosed ();
+		}
+	}
+
+	class ExtCommObj2 : ExtCommObj
+	{
+		public bool OnClosedCalled;
+
+		// It does not call base -> Abort() detects it as an error.
+		protected override void OnClosed ()
+		{
+			if (OnClosedCalled)
+				throw new Exception ("OnClosed() already called");
+			OnClosedCalled = true;
+		}
 	}
 
 	[TestFixture]
@@ -127,12 +148,33 @@ namespace MonoTests.System.ServiceModel.Channels
 		}
 
 		[Test]
-		[Category ("NotWorking")]
 		public void CloseAtInitialState ()
 		{
 			ExtCommObj obj = new ExtCommObj ();
-			obj.Close (); // Aborted() is called.
-			Assert.IsTrue (obj.Aborted);
+			obj.Close ();
+			Assert.IsTrue (obj.Aborted, "#1"); // OnAbort() is called.
+			Assert.IsFalse (obj.Closed, "#2"); // OnClose() is *not* called.
+			Assert.IsTrue (obj.OnClosedCalled, "#3");
+		}
+
+		[Test]
+		public void CloseAtInitialStateAsync ()
+		{
+			ExtCommObj obj = new ExtCommObj ();
+			obj.EndClose (obj.BeginClose (null, null)); // does not call OnBeginClose() / OnEndClose().
+			Assert.IsTrue (obj.Aborted, "#1");
+			Assert.IsFalse (obj.Closed, "#2");
+			Assert.IsTrue (obj.OnClosedCalled, "#3");
+		}
+
+		[Test]
+		public void CloseAtOpenedState ()
+		{
+			ExtCommObj obj = new ExtCommObj ();
+			obj.Open ();
+			obj.Close (); // Aborted() is *not* called.
+			Assert.IsFalse (obj.Aborted, "#1");
+			Assert.IsTrue (obj.Closed, "#2");
 		}
 
 		[Test]
@@ -154,8 +196,8 @@ namespace MonoTests.System.ServiceModel.Channels
 			obj = new ExtCommObj ();
 			obj.Open ();
 			obj.XFault ();
-			Assert.AreEqual (CommunicationState.Faulted, obj.State);
-			Assert.AreEqual (false, obj.IsDisposed);
+			Assert.AreEqual (CommunicationState.Faulted, obj.State, "#1");
+			Assert.AreEqual (false, obj.IsDisposed, "#2");
 		}
 
 		[Test]
@@ -176,5 +218,36 @@ namespace MonoTests.System.ServiceModel.Channels
 			obj.XFault ();
 			obj.Close ();
 		}
+
+		[Test]
+		public void AbortFaulted ()
+		{
+			ExtCommObj obj = new ExtCommObj ();
+			obj.Open ();
+			obj.XFault ();
+			Assert.AreEqual (CommunicationState.Faulted, obj.State, "#1");
+			obj.Abort (); // does not raise an error
+			Assert.AreEqual (CommunicationState.Closed, obj.State, "#2");
+			Assert.IsTrue (obj.Aborted, "#3");
+			Assert.IsFalse (obj.Closed, "#4");
+			obj.Abort (); // does not raise an error!
+		}
+
+		[Test]
+		public void AbortCreated ()
+		{
+			ExtCommObj obj = new ExtCommObj ();
+			obj.Abort ();
+			Assert.IsTrue (obj.Aborted, "#1"); // OnAbort() is called.
+			Assert.IsFalse (obj.Closed, "#2"); // OnClose() is *not* called.
+		}
+
+		[Test]
+		[ExpectedException (typeof (InvalidOperationException))]
+		public void OnClosedImplementedWithoutCallingBase ()
+		{
+			ExtCommObj obj = new ExtCommObj2 ();
+			obj.Close ();
+		}
 	}
 }