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

Add notes about closing streams after use (#3128) #3129

Closed
wants to merge 1 commit into from

Conversation

andrewnicols
Copy link

@andrewnicols andrewnicols commented May 4, 2023

As noted in #3128, it would be helpful to add a note about the need to close streams after using them. Failure to do so can cause issues, particularly on Windows.

@GrahamCampbell
Copy link
Member

Streams are, in general, closed automatically https://github.com/guzzle/psr7/blob/2.5.0/src/Stream.php#L69-L75, unless they stay referenced by your code or a previous response middleware.

@andrewnicols
Copy link
Author

andrewnicols commented May 8, 2023

Hi @GrahamCampbell ,

Thanks for your reply on this.

Whilst this may be the intent, something is preventing this from working properly on Windows. See the following testcase and it's GHA Run.

In this test I'm creating a file using file_put_contents(), obtaining a file handle, and then using a Guzzle Client to make a Request. I use Utils::streamFor(fopen(...)) to create the handle and do not store any reference to either the stream, or the file handle.

The first time that the test runs, everything works correctly.
Any subsequent time, it is not possible to create the file using file_put_contents. No reference is kept to either the stream, or the client, within the test code. I believe that happens because the file handle is still open to it.

I've added a second test to the same testcase which is almost identical except that a reference is stored, and is explicitly closed. This test passes on every iteration.

I would also argue that the intended behaviour also should be documented. That is, that a note should be added to explain that the file handle will be closed when the reference to the stream is unset and that, if you keep a handle to the stream, then it will not be closed until you dispose of the handle.

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.

None yet

2 participants