Fix 'Items already exist for target' build failure.

This patch addresses a fatal error and potential node crash in MSBuild
in a very specific condition.

Consider project A builds project B, C, B (using MSBuild task), project
B builds project C, and project C produces an error. In a multi-proc
build, the master node may build A, start building B and yield execution
for C to be built (on another node). On another node, the dependencies
of B are built and failed, returning the results to the master node.
When the execution of B resumes, the task building C is marked as failed
and execution continues. The master node then attempts to build B in a
second build request, sees the failure of C, and encounters an error
trying to set the result of the error target (target result already
exists).

This change calls the CheckSkipTarget method to determine if the target
has already been built in the case of an error. Previously this check
was made when checking dependencies of the target to inject or executing
the target. I believe this was an oversight in the original change that
was made to address this issue in a previous release of Visual Studio.

Note: The scenario described is implemented in unit test
VerifyMultipleRequestForSameProjectWithErrors_ErrorAndContinue. However,
since this issue causes a node to crash the unit test written did not
detect the more serious exception since the build was intended to fail
anyway. With this change, the test now passes.

Closes dotnet#1808

src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

@@ -474,21 +474,27 @@ private async Task ProcessTargetStack(ITaskBuilder taskBuilder)
                         break;
 
                     case TargetEntryState.ErrorExecution:
-                        // Push the error targets onto the stack.  This target will now be marked as completed.
-                        // When that state is processed, it will mark its parent for error execution
-                        List<TargetSpecification> errorTargets = currentTargetEntry.GetErrorTargets(_projectLoggingContext);
-                        try
-                        {
-                            await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, true, false, TargetPushType.Normal);
-                        }
-                        catch
+                        if (!CheckSkipTarget(ref stopProcessingStack, currentTargetEntry))
                         {
-                            if (_requestEntry.RequestConfiguration.ActivelyBuildingTargets.ContainsKey(currentTargetEntry.Name))
+                            // Push the error targets onto the stack.  This target will now be marked as completed.
+                            // When that state is processed, it will mark its parent for error execution
+                            var errorTargets = currentTargetEntry.GetErrorTargets(_projectLoggingContext);
+                            try
                             {
-                                _requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry.Name);
+                                await PushTargets(errorTargets, currentTargetEntry, currentTargetEntry.Lookup, true,
+                                    false, TargetPushType.Normal);
+                            }
+                            catch
+                            {
+                                if (_requestEntry.RequestConfiguration.ActivelyBuildingTargets.ContainsKey(
+                                    currentTargetEntry.Name))
+                                {
+                                    _requestEntry.RequestConfiguration.ActivelyBuildingTargets.Remove(currentTargetEntry
+                                        .Name);
+                                }
+
+                                throw;
                             }
-
-                            throw;
                         }
 
                         break;