Prevent stack overflow when verifying cyclic mock object graph

CHANGELOG.md

@@ -10,6 +10,11 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 * New SetupSequence verbs .PassAsync() and .ThrowsAsync(...) for async methods with void return type (@fuzzybair, #993)
 
+#### Fixed
+
+* `StackOverflowException` on `VerifyAll` when mocked method returns mocked object (@hotchkj, #1012)
+
+
 ## 4.14.0 (2020-04-24)
 
 #### Added

src/Moq/Mock.cs

@@ -295,14 +295,24 @@ public void VerifyAll()
 
 		private void Verify(Func<ISetup, bool> predicate)
 		{
-			if (!this.TryVerify(predicate, out var error) && error.IsVerificationError)
+			var verifiedMocks = new HashSet<Mock>();
+
+			if (!this.TryVerify(predicate, verifiedMocks, out var error) && error.IsVerificationError)
 			{
 				throw error;
 			}
 		}
 
-		internal bool TryVerify(Func<ISetup, bool> predicate, out MockException error)
+		internal bool TryVerify(Func<ISetup, bool> predicate, HashSet<Mock> verifiedMocks, out MockException error)
 		{
+			if (verifiedMocks.Add(this) == false)
+			{
+				// This mock has already been verified; don't verify it again.
+				// (We can end up here e.g. when there are loops in the inner mock object graph.)
+				error = null;
+				return true;
+			}
+
 			foreach (Invocation invocation in this.MutableInvocations)
 			{
 				invocation.MarkAsVerifiedIfMatchedBy(predicate);
@@ -312,7 +322,7 @@ internal bool TryVerify(Func<ISetup, bool> predicate, out MockException error)
 
 			foreach (var setup in this.MutableSetups.ToArray(predicate))
 			{
-				if (predicate(setup) && !setup.TryVerify(recursive: true, predicate, out var e) && e.IsVerificationError)
+				if (predicate(setup) && !setup.TryVerify(recursive: true, predicate, verifiedMocks, out var e) && e.IsVerificationError)
 				{
 					errors.Add(e);
 				}

src/Moq/Setup.cs

@@ -2,6 +2,7 @@
 // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq.Expressions;
 using System.Reflection;
@@ -127,6 +128,9 @@ public override string ToString()
 		/// <param name="predicate">
 		///   Specifies which setups should be verified.
 		/// </param>
+		/// <param name="verifiedMocks">
+		///   The set of mocks that have already been verified.
+		/// </param>
 		/// <param name="error">
 		///   If this setup or any of its inner mock (if present and known) failed verification,
 		///   this <see langword="out"/> parameter will receive a <see cref="MockException"/> describing the verification error(s).
@@ -135,7 +139,7 @@ public override string ToString()
 		///   <see langword="true"/> if verification succeeded without any errors;
 		///   otherwise, <see langword="false"/>.
 		/// </returns>
-		public bool TryVerify(bool recursive, Func<ISetup, bool> predicate, out MockException error)
+		internal bool TryVerify(bool recursive, Func<ISetup, bool> predicate, HashSet<Mock> verifiedMocks, out MockException error)
 		{
 			MockException e;
 
@@ -147,7 +151,7 @@ public bool TryVerify(bool recursive, Func<ISetup, bool> predicate, out MockExce
 			}
 
 			// optionally verify setups of inner mock (if present and known):
-			if (recursive && this.InnerMock?.TryVerify(predicate, out e) == false && e.IsVerificationError)
+			if (recursive && this.InnerMock?.TryVerify(predicate, verifiedMocks, out e) == false && e.IsVerificationError)
 			{
 				error = MockException.FromInnerMockOf(this, e);
 				return false;
@@ -186,12 +190,14 @@ public void VerifyAll()
 
 		private void Verify(bool recursive, Func<ISetup, bool> predicate)
 		{
+			var verifiedMocks = new HashSet<Mock>();
+
 			foreach (Invocation invocation in this.mock.MutableInvocations)
 			{
 				invocation.MarkAsVerifiedIfMatchedBy(setup => setup == this);
 			}
 
-			if (!this.TryVerify(recursive, predicate, out var error) && error.IsVerificationError)
+			if (!this.TryVerify(recursive, predicate, verifiedMocks, out var error) && error.IsVerificationError)
 			{
 				throw error;
 			}

tests/Moq.Tests/Regressions/IssueReportsFixture.cs

@@ -3279,6 +3279,40 @@ public interface IQueryOver<T>
 
 		#endregion
 
+		#region 1012
+
+		public class Issue1012
+		{
+			[Fact]
+			public void Verification_can_deal_with_cycles_in_inner_mock_object_graph_1()
+			{
+				var mock = new Mock<IX>();
+				mock.Setup(m => m.X).Returns(mock.Object);
+				_ = mock.Object.X;
+				mock.VerifyAll();
+			}
+
+			[Fact]
+			public void Verification_can_deal_with_cycles_in_inner_mock_object_graph_2()
+			{
+				var mock = new Mock<IX>();
+				var otherMock = new Mock<IX>();
+				mock.Setup(m => m.X).Returns(otherMock.Object);
+				otherMock.Setup(m => m.X).Returns(mock.Object);
+				_ = mock.Object.X;
+				_ = otherMock.Object.X;
+				mock.VerifyAll();
+				otherMock.VerifyAll();
+			}
+
+			public interface IX
+			{
+				IX X { get; }
+			}
+		}
+
+		#endregion
+
 		// Old @ Google Code
 
 		#region #47