Sync RuntimeProcess with upstream Eclipse platform - #745
Merged
robstryker merged 5 commits intoAug 20, 2026
Conversation
Port improvements from eclipse.platform's RuntimeProcess.java that accumulated since the original ~2015 fork. Fixes all three SpotBugs findings (IS2_INCONSISTENT_SYNC, SIC_INNER_SHOULD_BE_STATIC, SC_START_IN_CTOR ordering) and brings in upstream bug fixes. Key changes: - Fix inconsistent synchronization on fExitValue (now always written inside synchronized block in terminate() and terminated()) - Replace sleep-polling termination with process.waitFor(timeout) - Terminate descendant processes on shutdown - Use ConcurrentHashMap for thread-safe attribute storage - Simplify ProcessMonitorThread: private, volatile fExit, no redundant fields, access outer class via RuntimeProcess.this - Make fields final where appropriate (fMonitor, fStreamsProxy, fName, fCaptureOutput) - Register process with launch before starting monitor thread (Bug 598) - Use Objects.requireNonNull/equals in setAttribute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ~160 lines of shell-out environment parsing (exec of cmd.exe/env with manual stdout parsing, Win9xME support) with System.getenv(), which upstream Eclipse's LaunchManager already uses. Fixes all three SpotBugs findings: - Make singleton constructor private - Change static cache fields to instance fields so synchronized(this) is the correct lock scope Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Exclude EI_EXPOSE_REP and EI_EXPOSE_REP2 from the api.dao package. These DTOs are JSON-RPC data transfer objects where mutable field exposure is by design — callers build up DTOs and hand them to the serializer, so defensive copies would break expected usage patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace platform-default charset usage with StandardCharsets.UTF_8 across 8 production files. Platform default encoding varies by OS and locale, which can cause data corruption when reading/writing text that contains non-ASCII characters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ES_COMPARING_STRINGS_WITH_EQ: use equals() instead of == in Path - SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR: private constructor and final instance field in RuntimeProcessEventManager - SIC_INNER_SHOULD_BE_STATIC: make NullStreamMonitor static inner class - URF_UNREAD_FIELD: remove unused provider field in StandardCommandHandler - WMI_WRONG_MAP_ITERATOR: use entrySet() instead of keySet()+get() in StandardCommandHandler, StandardVMDebugger, ProcessUtility - SF_SWITCH_NO_DEFAULT: add default cases in ServerManagementClientImpl Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Aug 20, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
RuntimeProcess.java, resynchronizing with the currenteclipse.platformversion while preserving RSP-specific adaptations (DebugPluginConstants, RuntimeProcessEventManager, String encoding)fExitValue, non-static inner classProcessMonitorThread, and constructor thread-start orderingprocess.waitFor(timeout)and adds descendant process terminationHashMaptoConcurrentHashMapfor thread safetyTest plan
mvn compileon launching bundle — passesmvn verifyon launching test bundle — all 9 tests pass🤖 Generated with Claude Code