remoting: Handle DLLImport failure of libpsrpclient (#5622)

Issue #4029 exposed two problems when failing to load libpsrpclient.

WSManAPIDataCommon.ctor was not identifying the binary that failed DLLImport; hindering diagnosability
PrioritySendDataCollection.Clear() would throw a NullReferenceException when called from the finalizer after an error path.
This change addresses both of the above:

Guard against null arrays in PrioritySendDataCollection.Clear() - called from finalizer in error paths.

Diagnosability: Log the DllNotFoundException in WSManAPIDataCommon ctor and also include it as the internal exception when throwing PSRemotingTransportException.

src/System.Management.Automation/engine/remoting/fanin/PriorityCollection.cs

@@ -46,9 +46,11 @@ namespace System.Management.Automation.Remoting
         // actual data store(s) to store priority based data and its
         // corresponding sync objects to provide thread safety.
         private SerializedDataStream[] _dataToBeSent;
+        // array of sync objects, one for each element in _dataToBeSent
+        private object[] _dataSyncObjects;
+
         // fragmentor used to serialize & fragment objects added to this collection.
         private Fragmentor _fragmentor;
-        private object[] _syncObjects;
 
         // callbacks used if no data is available at any time.
         // these callbacks are used to notify when data becomes available under
@@ -95,11 +97,11 @@ namespace System.Management.Automation.Remoting
                 // create serialized streams using fragment size.
                 string[] names = Enum.GetNames(typeof(DataPriorityType));
                 _dataToBeSent = new SerializedDataStream[names.Length];
-                _syncObjects = new object[names.Length];
+                _dataSyncObjects = new object[names.Length];
                 for (int i = 0; i < names.Length; i++)
                 {
                     _dataToBeSent[i] = new SerializedDataStream(_fragmentor.FragmentSize);
-                    _syncObjects[i] = new object();
+                    _dataSyncObjects[i] = new object();
                 }
             }
         }
@@ -126,7 +128,7 @@ namespace System.Management.Automation.Remoting
             // make sure the only one object is fragmented and added to the collection
             // at any give time. This way the order of fragment is maintained
             // in the SendDataCollection(s).
-            lock (_syncObjects[(int)priority])
+            lock (_dataSyncObjects[(int)priority])
             {
                 _fragmentor.Fragment<T>(data, _dataToBeSent[(int)priority]);
             }
@@ -154,15 +156,33 @@ namespace System.Management.Automation.Remoting
         /// </summary>
         internal void Clear()
         {
-            Dbg.Assert(null != _dataToBeSent, "Serialized streams are not initialized");
-            lock (_syncObjects[(int)DataPriorityType.PromptResponse])
+            /*
+                NOTE: Error paths during initialization can cause _dataSyncObjects to be null
+                causing an unhandled exception in finalize and a process crash.
+                Verify arrays and dataToBeSent objects before referencing.
+            */
+            if (null != _dataSyncObjects && null != _dataToBeSent)
             {
-                _dataToBeSent[(int)DataPriorityType.PromptResponse].Dispose();
-            }
+                int promptResponseIndex = (int)DataPriorityType.PromptResponse;
+                int defaultIndex = (int)DataPriorityType.Default;
 
-            lock (_syncObjects[(int)DataPriorityType.Default])
-            {
-                _dataToBeSent[(int)DataPriorityType.Default].Dispose();
+                lock (_dataSyncObjects[promptResponseIndex])
+                {
+                    if (null != _dataToBeSent[promptResponseIndex])
+                    {
+                        _dataToBeSent[promptResponseIndex].Dispose();
+                        _dataToBeSent[promptResponseIndex] = null;
+                    }
+                }
+
+                lock (_dataSyncObjects[defaultIndex])
+                {
+                    if (null != _dataToBeSent[defaultIndex])
+                    {
+                        _dataToBeSent[defaultIndex].Dispose();
+                        _dataToBeSent[defaultIndex] = null;
+                    }
+                }
             }
         }
 
@@ -895,4 +915,4 @@ namespace System.Management.Automation.Remoting
     }
 
     #endregion
-}
+}

src/System.Management.Automation/engine/remoting/fanin/WSManTransportManager.cs

@@ -2602,9 +2602,19 @@ namespace System.Management.Automation.Remoting.Client
                 {
                     ErrorCode = WSManNativeApi.WSManInitialize(WSManNativeApi.WSMAN_FLAG_REQUESTED_API_VERSION_1_1, ref _handle);
                 }
-                catch (DllNotFoundException)
+                catch (DllNotFoundException ex)
                 {
-                    throw new PSRemotingTransportException(RemotingErrorIdStrings.WSManClientDllNotAvailable);
+                    PSEtwLog.LogOperationalError(
+                        PSEventId.TransportError,
+                        PSOpcode.Open,
+                        PSTask.None,
+                        PSKeyword.UseAlwaysOperational,
+                        "WSManAPIDataCommon.ctor",
+                        "WSManInitialize",
+                        ex.HResult.ToString(CultureInfo.InvariantCulture),
+                        ex.Message,
+                        ex.StackTrace);
+                    throw new PSRemotingTransportException(RemotingErrorIdStrings.WSManClientDllNotAvailable, ex);
                 }
 
                 // input / output streams common to all connections