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

I/O operation on closed file #1664

Closed
serhiy-storchaka opened this issue Sep 1, 2020 · 5 comments
Closed

I/O operation on closed file #1664

serhiy-storchaka opened this issue Sep 1, 2020 · 5 comments
Labels
T: bug Something isn't working

Comments

@serhiy-storchaka
Copy link

serhiy-storchaka commented Sep 1, 2020

Running Black 20.8b1 with the --diff option on a tree (> 100 files) produces errors like

error: cannot format tests/api/test_core.py: I/O operation on closed file

Running black --diff with a single file does not produce this error.

Linux/Python 3.7.8+

@serhiy-storchaka serhiy-storchaka added the T: bug Something isn't working label Sep 1, 2020
enzbang added a commit to enzbang/e3-core that referenced this issue Sep 2, 2020
Black fails with "I/O operation on closed file", see the
reported issue on black: psf/black#1664
@jpgrayson
Copy link
Contributor

As another data point from another code base, I observed this issue when 17 of 188 files would be changed by black. Once those files were changed to satisfy black, the issue went away.

When only 5 of 188 files do not satisfy black, the problem occurs sporadically for me.

So it seems like the problem manifests when there are a sufficient number of diffs being generated.

@ichard26 ichard26 added the stable label Sep 2, 2020
@jpgrayson
Copy link
Contributor

The problematic behavior only happens when the optional colorama package is installed.

When wrapping the output stream with colorama, it is assumed that colorama returns a wrapped stream without its own detach() method, but a wrapped stream is only returned on Windows platforms. The problem is that when no wrapper is affected, the code still unconditionally reassigns the original stream's detach() method to be a no-op.

I'll post a PR for this shortly.

jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 2, 2020
The wrap_stream_for_windows() function calls
colorama.initialise.wrap_stream() function to apply colorama's magic to
wrapper to the output stream. Except this wrapper is only applied on
Windows platforms that need it, otherwise the original stream is
returned as-is.

The colorama wrapped stream lacks a detach() method, so a no-op lambda
was being assigned to the wrapped stream.

The problem is that the no-op lambda was being assigned unconditionally
whether or not colorama actually returns a wrapped stream, thus
replacing the original TextIOWrapper's detach() method. Replacing the
detach() method with a no-op lambda is the root cause of the problem
observed in psf#1664.

The solution is to only assign the no-op detach() method if the stream
lacks its own detach() method.

Repairs psf#1664
jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 8, 2020
The wrap_stream_for_windows() function calls
colorama.initialise.wrap_stream() function to apply colorama's magic to
wrapper to the output stream. Except this wrapper is only applied on
Windows platforms that need it, otherwise the original stream is
returned as-is.

The colorama wrapped stream lacks a detach() method, so a no-op lambda
was being assigned to the wrapped stream.

The problem is that the no-op lambda was being assigned unconditionally
whether or not colorama actually returns a wrapped stream, thus
replacing the original TextIOWrapper's detach() method. Replacing the
detach() method with a no-op lambda is the root cause of the problem
observed in psf#1664.

The solution is to only assign the no-op detach() method if the stream
lacks its own detach() method.

Repairs psf#1664
jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 8, 2020
jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 8, 2020
jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 15, 2020
The wrap_stream_for_windows() function calls
colorama.initialise.wrap_stream() function to apply colorama's magic to
wrapper to the output stream. Except this wrapper is only applied on
Windows platforms that need it, otherwise the original stream is
returned as-is.

The colorama wrapped stream lacks a detach() method, so a no-op lambda
was being assigned to the wrapped stream.

The problem is that the no-op lambda was being assigned unconditionally
whether or not colorama actually returns a wrapped stream, thus
replacing the original TextIOWrapper's detach() method. Replacing the
detach() method with a no-op lambda is the root cause of the problem
observed in psf#1664.

The solution is to only assign the no-op detach() method if the stream
lacks its own detach() method.

Repairs psf#1664
jpgrayson added a commit to jpgrayson/black that referenced this issue Sep 15, 2020
zsol pushed a commit that referenced this issue Sep 27, 2020
* Repair colorama wrapping on non-Windows platforms

The wrap_stream_for_windows() function calls
colorama.initialise.wrap_stream() function to apply colorama's magic to
wrapper to the output stream. Except this wrapper is only applied on
Windows platforms that need it, otherwise the original stream is
returned as-is.

The colorama wrapped stream lacks a detach() method, so a no-op lambda
was being assigned to the wrapped stream.

The problem is that the no-op lambda was being assigned unconditionally
whether or not colorama actually returns a wrapped stream, thus
replacing the original TextIOWrapper's detach() method. Replacing the
detach() method with a no-op lambda is the root cause of the problem
observed in #1664.

The solution is to only assign the no-op detach() method if the stream
lacks its own detach() method.

Repairs #1664
@ichard26
Copy link
Collaborator

Running black -v --diff -l 79 over some typeshed stubs:

Before 4d71d74:

2 files would be reformatted, 56 files would be left unchanged, 86 files would fail to reformat.

After 4d71d74:

88 files would be reformatted, 56 files would be left unchanged.
My environment
  • Python version: CPython 3.8.5
  • Black version: look above
  • OS version: Ubuntu 20.04.01 LTS

I'll have to check that the patch also works on Windows but that will take a while - hopefully I don't have mess around with Windows Update :-)

@ichard26
Copy link
Collaborator

I'll have to check that the patch also works on Windows but that will take a while - hopefully I don't have mess around with Windows Update :-)

I procrastinated for way too long on getting back if this patch works on Windows or not but the answer is "yes it does" :D

@ichard26
Copy link
Collaborator

This was fixed quite a while ago, although it was only released in 21.4b0.

noxan pushed a commit to noxan/black that referenced this issue Jun 6, 2021
* Repair colorama wrapping on non-Windows platforms

The wrap_stream_for_windows() function calls
colorama.initialise.wrap_stream() function to apply colorama's magic to
wrapper to the output stream. Except this wrapper is only applied on
Windows platforms that need it, otherwise the original stream is
returned as-is.

The colorama wrapped stream lacks a detach() method, so a no-op lambda
was being assigned to the wrapped stream.

The problem is that the no-op lambda was being assigned unconditionally
whether or not colorama actually returns a wrapped stream, thus
replacing the original TextIOWrapper's detach() method. Replacing the
detach() method with a no-op lambda is the root cause of the problem
observed in psf#1664.

The solution is to only assign the no-op detach() method if the stream
lacks its own detach() method.

Repairs psf#1664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants