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: MSVC issue #2823

Merged
merged 2 commits into from
Jan 27, 2021
Merged

fix: MSVC issue #2823

merged 2 commits into from
Jan 27, 2021

Conversation

henryiii
Copy link
Collaborator

@henryiii henryiii commented Jan 27, 2021

Attempt to fix issue seen in #2821. Pulled out the non-changelog part of that PR too (adding pytest-timeout, corrected mypy config).

CC @YannickJadoul @rwgk @bstaletic

@YannickJadoul
Copy link
Collaborator

Cross-linking to #2754

include/pybind11/iostream.h Outdated Show resolved Hide resolved
@henryiii henryiii merged commit 732bf88 into pybind:master Jan 27, 2021
@henryiii henryiii deleted the fix/win branch January 27, 2021 01:59
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jan 27, 2021
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Jan 27, 2021
@rwgk
Copy link
Collaborator

rwgk commented Jan 27, 2021

I didn't follow the conversations enough to know if this PR was expected to also fix the TSAN error reported under #2754, but I just tried, and I still see this with current master:

  WARNING: ThreadSanitizer: data race llvm/llvm-project/libcxx/include/streambuf:462:13 in std::__tsan::basic_streambuf<char, std::__tsan::char_traits<char> >::xsputn(char const*, long) (pybind11/tests/test_iostream)
  WARNING: ThreadSanitizer: data race llvm/llvm-project/libcxx/include/__string:388:56 in std::__tsan::char_traits<char>::copy(char*, char const*, unsigned long) (pybind11/tests/test_iostream)

@henryiii
Copy link
Collaborator Author

I saw the output issue again, but at least the hangup stopped, and the issue is rarer than it used to be. Issue was in one job out of 5-6 sets of builds.

@YannickJadoul
Copy link
Collaborator

Hmmm, so we're still missing something? :-(

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.

4 participants