This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Cygwin: heap corruption bug in wcsxfrm
Type: Stage: resolved
Components: Versions:
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: erik.bray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-10-27 13:42 by erik.bray, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4151 closed erik.bray, 2017-10-27 13:45
Messages (7)
msg305120 - (view) Author: Erik Bray (erik.bray) * (Python triager) Date: 2017-10-27 13:42
There is an acknowledged bug in Cygwin's implementation of wcsxfrm() [1] that can cause heap corruption in certain cases.  This bug has since been fixed in Cygwin 2.8.1-1 [2] and all current and future releases.  However, that was relatively recent (July 2017) so it may still crop up.

I also have a workaround for this from the Python side, but rather than clutter the code with workarounds for platform-specific bugs I think it suffices just to skip the test in this case.

[1] https://cygwin.com/ml/cygwin/2017-05/msg00149.html
[2] https://cygwin.com/ml/cygwin-announce/2017-07/msg00002.html
msg305126 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-27 16:56
Since this bug has been fixed in Cygwin, I don't think we should add a workaround for it. The expected date of Python 3.7 release is 2018-06-15, this is far from the date of releasing the fixed Cygwin.
msg305131 - (view) Author: Erik Bray (erik.bray) * (Python triager) Date: 2017-10-27 18:20
To be clear, are you saying there shouldn't be a workaround to the underlying issue (I agree), or are you saying the test skip shouldn't even be added? I'm in favor of just skipping the test since it's still a problem on (currently) recent Cygwin versions. And it's not a very critical test so I'm happy to just skip it in this case.
msg305133 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-27 19:42
According to your reference a problem is fixed in recent Cygwin versions, isn't?

Skipping the test on Cygwin means that wcsxfrm() will be untested on this platform. Future regressions will be unnoticed.
msg305215 - (view) Author: Erik Bray (erik.bray) * (Python triager) Date: 2017-10-30 10:05
Well, I agree there's an unfortunate trade-off here: One can skip the test, allowing the test suite to work on slightly older versions of Cygwin, from before this issue was fixed.  However, I then lose regression testing on newer versions.  My personal feeling in this case is that it's not a very important test and can be skipped in the future (although the bug that this test caught was fairly serious and one would want regression testing for it...)

As an alternative I could be more fine-grained and only skip the test on older versions of Cygwin that are affected.  I was hoping to avoid putting in too much Cygwin-specific test utilities, though adding a version check for Cygwin is not terribly hard (I do the same for my Cygwin port of psutil).

For reference, the current version of Cygwin that comes installed on AppVeyor (for which I'm trying to get a Cygwin build set up) is 2.8.0, which *is* (just barely) affected by this bug.
msg305217 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-30 10:14
This is a problem of AppVeyor. It should update Cygwin to a bugfix release. 2.8.0 contains other bugs.
msg305224 - (view) Author: Erik Bray (erik.bray) * (Python triager) Date: 2017-10-30 11:38
Well, we'll see how long it takes me to get them to respond on that.  If it's not too much trouble then I'll be happy to drop this issue.
History
Date User Action Args
2022-04-11 14:58:53adminsetgithub: 76064
2017-10-30 11:38:25erik.braysetmessages: + msg305224
2017-10-30 10:14:51serhiy.storchakasetstatus: open -> closed
resolution: out of date
messages: + msg305217

stage: patch review -> resolved
2017-10-30 10:05:08erik.braysetmessages: + msg305215
2017-10-27 19:42:06serhiy.storchakasetmessages: + msg305133
2017-10-27 18:20:03erik.braysetmessages: + msg305131
2017-10-27 16:56:30serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg305126
2017-10-27 13:45:28erik.braysetkeywords: + patch
stage: patch review
pull_requests: + pull_request4116
2017-10-27 13:42:11erik.braycreate