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/full invert crash 2942 #2957

Merged

Conversation

Abdurrahman-shaikh
Copy link
Contributor

Description of Changes

Please provide a summary of the changes, including:

  • What was changed

    • Modified the convertToBufferedImageTpFile to use File.createTempFile() instead of writing to "image.png" in the current directory.
    • This change ensures the file is saved in the default temporary directory, preventing permission issues.
  • Why the change was made

    • Previously, the method attempted to save the file in the current working directory, which caused permission errors (java.io.FileNotFoundException: image.png (Permission denied)).
  • Any challenges encountered

Closes #2942


Checklist

General

Documentation

UI Changes (if applicable)

  • Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR)

Testing (if applicable)

  • I have tested my changes locally. Refer to the Testing Guide for more details.

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. Bug Something isn't working labels Feb 16, 2025
@github-actions github-actions bot added the Java Pull requests that update Java code label Feb 16, 2025
ImageIO.write(image, "png", file);
//Ensures temp file is deleted when jvm terminates
file.deleteOnExit();
Copy link
Member

Choose a reason for hiding this comment

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

Delete on exit isn't enough since this server could be running for weeks or months, we must delete it after operation
Can you replace this with a try finally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete on exit isn't enough since this server could be running for weeks or months, we must delete it after operation Can you replace this with a try finally?

Ok, I will replace it with a try-finally block

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 16, 2025
@Abdurrahman-shaikh
Copy link
Contributor Author

@Frooodle I’ve updated the code to address your feedback. Temporary files are now cleaned up immediately using try-finally blocks. Could you please review the changes?

@Frooodle Frooodle merged commit 82b1ab4 into Stirling-Tools:main Feb 17, 2025
8 of 9 checks passed
@Abdurrahman-shaikh Abdurrahman-shaikh deleted the fix/Full-Invert-Crash-2942 branch February 17, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Java Pull requests that update Java code size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Full-Invert crash
2 participants