Skip to content

Conversation

HalfWhitt
Copy link
Member

Currently, when core's Image() is given a path, it passes that path on to the backend. We've learned (from a report on Discord) that, at least on Windows, this results in the file being kept open even after the Image is created (presumably by something in Python.NET). This PR is to change the logic so that core handles reading the file's data, then passes that along to the backend. This way we can ensure the file is closed.

I'm uploading this first as just the test, because I'm curious to see which platforms currently suffer from this issue.

Also: this will obsolete the path parameter to each backend's Image class. Is that worth a deprecation, since it's internal and version-locked?

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@HalfWhitt
Copy link
Member Author

Interesting. All the testbeds pass, even on Windows, which is where the report originally came from. Either my test's faulty or the behavior observed involves something else at play. Time to fire up the VM and experiment.

Comment on lines +26 to +30
async def test_closed_file_handle(app):
"""The local image file isn't left open once the Image is created."""
_ = toga.Image("resources/sample.png")
# If the file still has an open handle, this should raise an IOError.
_ = Path(app.paths.app / "resources/sample.png").read_bytes()
Copy link
Member

Choose a reason for hiding this comment

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

pardon the interjection....but reading the file should be fine; however, deleting the file here should at least cause problems on Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, hm. That makes sense. I suppose I could make a copy, and delete it afterward.

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.

2 participants