[GEODE-10522] Security : Eliminate Reflection in VMStats50 to Remove --add-opens Requirement #7957
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,17 +20,17 @@ | |
| import java.lang.management.MemoryMXBean; | ||
| import java.lang.management.MemoryPoolMXBean; | ||
| import java.lang.management.MemoryUsage; | ||
| import java.lang.management.ThreadInfo; | ||
| import java.lang.management.ThreadMXBean; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.Iterator; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import com.sun.management.OperatingSystemMXBean; | ||
| import com.sun.management.UnixOperatingSystemMXBean; | ||
| import org.apache.logging.log4j.Logger; | ||
|
|
||
| import org.apache.geode.StatisticDescriptor; | ||
|
|
@@ -41,7 +41,6 @@ | |
| import org.apache.geode.SystemFailure; | ||
| import org.apache.geode.annotations.Immutable; | ||
| import org.apache.geode.annotations.internal.MakeNotStatic; | ||
| import org.apache.geode.internal.statistics.StatisticsTypeFactoryImpl; | ||
| import org.apache.geode.internal.statistics.VMStatsContract; | ||
| import org.apache.geode.logging.internal.log4j.api.LogService; | ||
|
|
@@ -61,20 +60,24 @@ public class VMStats50 implements VMStatsContract { | |
| private static final ClassLoadingMXBean clBean; | ||
| @Immutable | ||
| private static final MemoryMXBean memBean; | ||
|
|
||
| /** | ||
| * Platform-specific OperatingSystemMXBean providing extended metrics beyond the standard | ||
| * java.lang.management.OperatingSystemMXBean. This interface is in the exported | ||
| * com.sun.management package and provides processCpuTime and other platform metrics. | ||
| * Available on all platforms. | ||
| */ | ||
| @Immutable | ||
| private static final OperatingSystemMXBean platformOsBean; | ||
|
|
||
| /** | ||
| * Unix-specific OperatingSystemMXBean providing file descriptor metrics. | ||
| * Only available on Unix-like platforms (Linux, macOS, Solaris, etc.). | ||
| * Gracefully null on Windows and other non-Unix platforms. | ||
| */ | ||
| @Immutable | ||
| private static final UnixOperatingSystemMXBean unixOsBean; | ||
|
|
||
| @Immutable | ||
| private static final ThreadMXBean threadBean; | ||
|
|
||
|
|
@@ -150,52 +153,50 @@ public class VMStats50 implements VMStatsContract { | |
| static { | ||
| clBean = ManagementFactory.getClassLoadingMXBean(); | ||
| memBean = ManagementFactory.getMemoryMXBean(); | ||
|
|
||
| // Initialize platform-specific MXBeans using direct interface casting. | ||
| // This approach eliminates the need for reflection and --add-opens flags. | ||
| // The com.sun.management package is exported by jdk.management module, | ||
| // making these interfaces accessible without module violations. | ||
| OperatingSystemMXBean tempPlatformBean = null; | ||
| UnixOperatingSystemMXBean tempUnixBean = null; | ||
|
|
||
| try { | ||
| // Get the standard OperatingSystemMXBean | ||
| java.lang.management.OperatingSystemMXBean stdOsBean = | ||
| ManagementFactory.getOperatingSystemMXBean(); | ||
|
|
||
| // Cast to com.sun.management.OperatingSystemMXBean for extended metrics | ||
| // This interface is in the exported com.sun.management package | ||
| if (stdOsBean instanceof OperatingSystemMXBean) { | ||
| tempPlatformBean = (OperatingSystemMXBean) stdOsBean; | ||
| } | ||
|
|
||
| // Check for Unix-specific interface | ||
| // This is only available on Unix-like platforms (Linux, macOS, Solaris) | ||
| if (stdOsBean instanceof UnixOperatingSystemMXBean) { | ||
| tempUnixBean = (UnixOperatingSystemMXBean) stdOsBean; | ||
| } | ||
| } catch (VirtualMachineError err) { | ||
| SystemFailure.initiateFailure(err); | ||
| // If this ever returns, rethrow the error. We're poisoned | ||
| // now, so don't let this thread continue. | ||
| throw err; | ||
| } catch (Throwable ex) { | ||
| // Whenever you catch Error or Throwable, you must also | ||
| // catch VirtualMachineError (see above). However, there is | ||
| // _still_ a possibility that you are dealing with a cascading | ||
| // error condition, so you also need to check to see if the JVM | ||
| // is still usable: | ||
| logger.warn("Unable to access platform OperatingSystemMXBean: {}", ex.getMessage()); | ||
| SystemFailure.checkFailure(); | ||
| tempPlatformBean = null; | ||
| tempUnixBean = null; | ||
| } finally { | ||
| platformOsBean = tempPlatformBean; | ||
| unixOsBean = tempUnixBean; | ||
| } | ||
|
|
||
| threadBean = ManagementFactory.getThreadMXBean(); | ||
| if (THREAD_STATS_ENABLED) { | ||
| if (threadBean.isThreadCpuTimeSupported()) { | ||
|
|
@@ -242,7 +243,7 @@ public class VMStats50 implements VMStatsContract { | |
| true)); | ||
| sds.add(f.createLongCounter("processCpuTime", "CPU timed used by the process in nanoseconds.", | ||
| "nanoseconds")); | ||
| if (unixOsBean != null) { | ||
| sds.add(f.createLongGauge("fdLimit", "Maximum number of file descriptors", "fds", true)); | ||
| sds.add(f.createLongGauge("fdsOpen", "Current number of open file descriptors", "fds")); | ||
| } | ||
|
|
@@ -260,7 +261,7 @@ public class VMStats50 implements VMStatsContract { | |
| totalMemoryId = vmType.nameToId("totalMemory"); | ||
| maxMemoryId = vmType.nameToId("maxMemory"); | ||
| processCpuTimeId = vmType.nameToId("processCpuTime"); | ||
| if (unixOsBean != null) { | ||
| unix_fdLimitId = vmType.nameToId("fdLimit"); | ||
| unix_fdsOpenId = vmType.nameToId("fdsOpen"); | ||
| } else { | ||
|
|
@@ -585,7 +586,7 @@ private void refreshGC() { | |
| public void refresh() { | ||
| Runtime rt = Runtime.getRuntime(); | ||
| vmStats.setInt(pendingFinalizationCountId, memBean.getObjectPendingFinalizationCount()); | ||
| vmStats.setInt(cpusId, platformOsBean.getAvailableProcessors()); | ||
|
||
| vmStats.setInt(threadsId, threadBean.getThreadCount()); | ||
| vmStats.setInt(daemonThreadsId, threadBean.getDaemonThreadCount()); | ||
| vmStats.setInt(peakThreadsId, threadBean.getPeakThreadCount()); | ||
|
|
@@ -596,32 +597,38 @@ public void refresh() { | |
| vmStats.setLong(totalMemoryId, rt.totalMemory()); | ||
| vmStats.setLong(maxMemoryId, rt.maxMemory()); | ||
|
|
||
| // Collect process CPU time using public com.sun.management API. | ||
| // No reflection or setAccessible() required - this is a properly | ||
| // exported interface method from the jdk.management module. | ||
| if (platformOsBean != null) { | ||
| try { | ||
| long cpuTime = platformOsBean.getProcessCpuTime(); | ||
| vmStats.setLong(processCpuTimeId, cpuTime); | ||
| } catch (VirtualMachineError err) { | ||
| SystemFailure.initiateFailure(err); | ||
| // If this ever returns, rethrow the error. We're poisoned | ||
| // now, so don't let this thread continue. | ||
| throw err; | ||
| } catch (Throwable ex) { | ||
| // Whenever you catch Error or Throwable, you must also | ||
| // catch VirtualMachineError (see above). However, there is | ||
| // _still_ a possibility that you are dealing with a cascading | ||
| // error condition, so you also need to check to see if the JVM | ||
| // is still usable: | ||
| SystemFailure.checkFailure(); | ||
| } | ||
| } | ||
|
|
||
| // Collect Unix-specific file descriptor metrics. | ||
| // This interface is only implemented on Unix-like platforms; | ||
| // gracefully null on Windows. | ||
| if (unixOsBean != null) { | ||
| try { | ||
| long maxFd = unixOsBean.getMaxFileDescriptorCount(); | ||
| vmStats.setLong(unix_fdLimitId, maxFd); | ||
|
|
||
| long openFd = unixOsBean.getOpenFileDescriptorCount(); | ||
| vmStats.setLong(unix_fdsOpenId, openFd); | ||
| } catch (VirtualMachineError err) { | ||
| SystemFailure.initiateFailure(err); | ||
| // If this ever returns, rethrow the error. We're poisoned | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider more specific error message indicating this affects statistics collection but not core functionality of Geode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent suggestion, @sboorlagadda. The enhanced error message now clearly communicates that this affects statistics collection but not core functionality, which will help operators understand the severity. Updated in the latest commit. Thank you for improving the operational clarity!