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

BlackDTestCase.test_cors_preflight failure on 22.8.0 #3251

Closed
polyzen opened this issue Sep 1, 2022 · 7 comments · Fixed by #4560
Closed

BlackDTestCase.test_cors_preflight failure on 22.8.0 #3251

polyzen opened this issue Sep 1, 2022 · 7 comments · Fixed by #4560
Labels
C: integrations Editor plugins and other integrations help wanted Extra attention is needed T: bug Something isn't working

Comments

@polyzen
Copy link
Contributor

polyzen commented Sep 1, 2022

Describe the bug

BlackDTestCase.test_cors_preflight test failure:

show/hide
______________________ BlackDTestCase.test_cors_preflight ______________________

self = <tests.test_blackd.BlackDTestCase testMethod=test_cors_preflight>

    async def get_application(self) -> web.Application:
>       return blackd.make_app()

tests/test_blackd.py:46:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test-env/lib/python3.10/site-packages/blackd/__init__.py:76: in make_app
    executor = ProcessPoolExecutor()
/usr/lib/python3.10/concurrent/futures/process.py:657: in __init__
    self._call_queue = _SafeQueue(
/usr/lib/python3.10/concurrent/futures/process.py:168: in __init__
    super().__init__(max_size, ctx=ctx)
/usr/lib/python3.10/multiprocessing/queues.py:42: in __init__
    self._reader, self._writer = connection.Pipe(duplex=False)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

duplex = False

    def Pipe(duplex=True):
        '''
        Returns pair of connection objects at either end of a pipe
        '''
        if duplex:
            s1, s2 = socket.socketpair()
            s1.setblocking(True)
            s2.setblocking(True)
            c1 = Connection(s1.detach())
            c2 = Connection(s2.detach())
        else:
>           fd1, fd2 = os.pipe()
E           OSError: [Errno 24] Too many open files

/usr/lib/python3.10/multiprocessing/connection.py:532: OSError
=========================== short test summary info ============================
FAILED tests/test_blackd.py::BlackDTestCase::test_cors_preflight - OSError: [...
================== 1 failed, 286 passed, 3 skipped in 11.32s ===================

To Reproduce

https://github.com/archlinux/svntogit-community/blob/8ef0305f6c9b28df533696a7e54d08269dc75aa8/trunk/PKGBUILD#L33-L35

python -m venv --system-site-packages test-env
test-env/bin/python -m installer dist/*.whl
PATH="$PWD/test-env/bin:$PATH" test-env/bin/python -m pytest

The package build process also hangs after the above output.

Expected behavior

Test passes and we all live happily ever after.

Environment

  • Black's version: 22.8.0
  • OS and Python version: Arch Linux/Python 3.10.6

Additional context

Can provide dependency versions if that would help.

@polyzen polyzen added the T: bug Something isn't working label Sep 1, 2022
@ichard26 ichard26 added C: integrations Editor plugins and other integrations help wanted Extra attention is needed labels Sep 1, 2022
@ichard26
Copy link
Collaborator

ichard26 commented Sep 1, 2022

Hmm, the OSError sounds like a host issue where some application or code is opening many files and isn't closing them. (Are there many other processes on the system?) I don't believe Black leaks file descriptors anywhere? Blackd could be leaking some, but I have no idea. ProcessPoolExecutor() is a pretty typical use so there shouldn't be anything wrong with that.

@polyzen
Copy link
Contributor Author

polyzen commented Sep 1, 2022

Hmm, the OSError sounds like a host issue where some application or code is opening many files and isn't closing them. (Are there many other processes on the system?)

It's a build server used for Arch Linux packages. The test passes and the build completes when done locally 👍.

@ichard26
Copy link
Collaborator

ichard26 commented Sep 4, 2022

Ah thanks for checking!

Is something on Black's end we can do to help with this, or can we close this issue as "check the downstream build server for FD overusage"?

@ichard26 ichard26 added the S: awaiting response Waiting for futher information from OP label Sep 4, 2022
@foutrelis
Copy link

The discrepancy between local and remote could be due to the higher thread count on the build box (64 threads). The default open file limit is 1024 and running the test suite with so many threads somehow reaches it. If a fix cannot be implemented upstream, a downstream workaround would be to ulimit -n 2048 before running the tests.

@ichard26
Copy link
Collaborator

ichard26 commented Sep 4, 2022

Alright thanks for the insights, I can try investigating later. It is surprising how a single pytest run would run into an open file limit of 1024.

@ichard26 ichard26 removed the S: awaiting response Waiting for futher information from OP label Sep 4, 2022
@foutrelis
Copy link

It is surprising how a single pytest run would run into an open file limit of 1024.

Indeed, which makes it more likely that open files from previous tests are involved. If I run only the 16 tests from tests/test_blackd.py, they all pass. The failure occurs when they run after the 110 tests from tests/test_black.py.

@saksham-sak

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: integrations Editor plugins and other integrations help wanted Extra attention is needed T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants