-
Notifications
You must be signed in to change notification settings - Fork 693
GEODE-10531: Remove SecurityManager Usage from OSProcess.java - Java 21 Blocker #7971
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
- Phase 0 (Critical Java 21 Blockers) is now complete - Updated warning baseline from 41 to 39 total warnings - Marked SecurityManager API removal as resolved - Added completion details and lessons learned - Updated project status to focus on Phase 1 (Spring Framework blockers)
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.
Pull request overview
This PR addresses GEODE-10531, removing deprecated SecurityManager APIs from OSProcess.java that were removed in Java 21, thereby unblocking the future migration to Java 21. The change is part of a larger initiative (GEODE-10479) to remove all deprecation warnings from the Apache Geode codebase following the Java 17 migration.
Key Changes:
- Removed SecurityManager references and checks from
OSProcess.bgexec()method - Updated JavaDoc to remove references to
SecurityManagerandSecurityManager#checkExec() - Security functionality now relies on OS/JVM security models rather than the deprecated SecurityManager API
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| proposals/GEODE-10479/todo.md | Tracks completion of Phase 0 critical Java 21 blocker (SecurityManager removal), marking task as complete with detailed implementation notes |
| proposals/GEODE-10479/spec.md | Technical specification document for the overall Java 17 deprecation warning removal project (new file) |
| proposals/GEODE-10479/plan.md | Comprehensive implementation plan for the deprecation removal project including prompts for LLM code generation (new file) |
| proposals/GEODE-10479/issue.md | JIRA-formatted issue description for the overall deprecation removal effort (new file) |
| proposals/GEODE-10479/GEODE-10534.md | Documentation for fixing 13 deprecation warnings across 4 support modules (new file) |
| proposals/GEODE-10479/GEODE-10533.md | Documentation for fixing 23 warnings in geode-gfsh module (new file) |
| proposals/GEODE-10479/GEODE-10532.md | Documentation for high-priority Spring Framework getRawStatusCode() removal warnings (new file) |
| proposals/GEODE-10479/GEODE-10531.md | Issue documentation for this specific SecurityManager removal task (new file) |
| geode-logging/src/main/java/org/apache/geode/logging/internal/OSProcess.java | Removed 6 lines containing SecurityManager API usage and JavaDoc references that block Java 21 compilation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
geode-logging/src/main/java/org/apache/geode/logging/internal/OSProcess.java
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JinwooHwang
left a comment
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.
Hi @sboorlagadda. Thank you for addressing this Java 21 blocker - removing the deprecated SecurityManager usage is definitely necessary for forward compatibility.
While I understand that SecurityManager removal is mandatory for Java 21 compilation, I'm concerned about completely removing the subprocess execution security check without providing any replacement mechanism. Since this check previously acted as a safeguard against unauthorized command invocation, could you clarify how equivalent protections will be maintained? If no replacement is planned, could you please help me understand the reasoning for why this execution path no longer requires any guardrail.
For all changes, please confirm:
develop)?gradlew buildrun cleanly?