Skip to content
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

fix: resolve SonarCloud issue in App.java #2865 #3236

Merged

Conversation

2897robo
Copy link
Contributor

@2897robo 2897robo commented Mar 31, 2025

What does this PR do?

This PR resolves a high severity issue (java:S2095) reported by SonarCloud in App.java.
The issue was that ScheduledExecutorService was not properly closed, potentially leading to resource leakage.

🔗 SonarCloud issue: https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AZXOQp-k7L73unJ7u3kj

What did I do?

  • Wrapped ScheduledExecutorService with try-finally to ensure proper shutdown
  • Added a Thread.sleep(45000) for demonstration purposes to allow task completion before shutdown
  • Confirmed locally with IntelliJ SonarLint (project bound to SonarCloud) that the warning is resolved
  • Verified successful analysis from SonarCloud (Quality Gate Passed)

References

  • Bound IntelliJ SonarLint to SonarCloud (see project wiki's IDE instructions)
  • Rule: java:S2095

Fixes #2865

Copy link

github-actions bot commented Mar 31, 2025

PR Summary

This PR resolves a SonarCloud issue in App.java by adding a try-finally block to ensure the ScheduledExecutorService is properly shut down, preventing resource leaks. A 45-second delay is added for demonstration purposes before the executor service is shut down.

Changes

File Summary
dirty-flag/src/main/java/com/iluwatar/dirtyflag/App.java The ScheduledExecutorService is now wrapped in a try-finally block to guarantee proper shutdown. A Thread.sleep(45000) is added for demonstration, allowing tasks to complete before shutdown. Error handling is improved with logging of interruptions.

autogenerated by presubmit.ai

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Review Summary

Commits Considered (1)
Files Processed (1)
  • dirty-flag/src/main/java/com/iluwatar/dirtyflag/App.java (1 hunk)
Actionable Comments (0)
Skipped Comments (1)
  • dirty-flag/src/main/java/com/iluwatar/dirtyflag/App.java [76-77]

    enhancement: "Remove sleep for production."

@2897robo
Copy link
Contributor Author

Hi @iluwatar,
This PR fixes a high severity issue reported by SonarCloud. I verified the fix locally using SonarLint bound to SonarCloud.
Kindly review when you get a chance 😊

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Review Summary

Commits Considered (1)
  • a8bffc1: Merge branch 'master' into fix/sonar-high-issue-dirty-flag-app
Files Processed (0)
Actionable Comments (0)
Skipped Comments (0)

@iluwatar iluwatar merged commit 9073b78 into iluwatar:master Mar 31, 2025
2 checks passed
@iluwatar
Copy link
Owner

Looks good! Thank you for the contribution 🎉

@all-contributors please add @2897robo for code

Copy link
Contributor

@iluwatar

I've put up a pull request to add @2897robo! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix high severity issues reported by SonarCloud
2 participants