malliaridis commented on code in PR #4718:
URL: https://github.com/apache/solr/pull/4718#discussion_r4199423952


##########
solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java:
##########
@@ -174,55 +173,45 @@ private static Optional<String> commandLine(ProcessHandle 
ph) {
   }
 
   /**
-   * Gets the command lines of all java processes on Windows using PowerShell.
+   * WMI columns to select from {@code Win32_Process}. The enum constant names 
are used verbatim as
+   * the WQL {@code SELECT} column names (WQL is case-insensitive).
+   */
+  enum ProcessProperty {
+    PROCESSID,
+    COMMANDLINE
+  }
+
+  /**
+   * Gets the command lines of all java processes on Windows by querying WMI 
({@code Win32_Process})
+   * through JNA. This avoids spawning an external PowerShell process.
    *
    * @return a map of process IDs to command lines
    */
   private static Map<Long, String> commandLinesWindows() {
+    COMUtils.checkRC(Ole32.INSTANCE.CoInitializeEx(null, 
Ole32.COINIT_MULTITHREADED));

Review Comment:
   Since I am not very familiar with the multi-threading behavior and all the 
COM-specific edge-cases, I let an agent take a look on this one. It mentioned a 
few scenarios that I cross-checked and were related to `COMUtils.checkRC()`. 
Only one issue exists, but likely can be ignored here:
   
   When there are multiple instances of ProcessManager initialized at the same 
time, one process manager may uninitialize Ole32.INSTANCE on finally of any 
other ProcessManager before they finish execution. But I believe that is 
unlikely to happen and not important(?). The agent suggested to add a guard of 
this kind:
   
   ```java
   HRESULT hr = Ole32.INSTANCE.CoInitializeEx(null, Ole32.COINIT_MULTITHREADED);
   // Only uninitialize if we got S_OK (0). If we got S_FALSE (1), someone else 
owns it.
   boolean coUninitializeRequired = hr.equals(W32Errors.S_OK); 
   
   try {
       // Run WMI query...
   } finally {
       if (coUninitializeRequired) {
           Ole32.INSTANCE.CoUninitialize();
       }
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to