Closed
Bug 1349912
Opened 7 years ago
Closed 7 years ago
Solve MinGW's std::thread problems
Categories
(Core :: General, enhancement)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: tjr, Assigned: tjr)
References
(Blocks 1 open bug)
Details
(Whiteboard: [tor])
Attachments
(1 file, 3 obsolete files)
MinGW has problems with std::thread. It has two threading models: win32 API based, which disables std::thread; and POSIX based which enables it but requires an emulation library (winpthreads). While a little bit ago in Bug 1317642 we were hoping to avoid this problem and hope no one used std::thread, it seems to be coming up more and more and we won't be able to remove them all. For example: Bug 1344909 uses it (although we could work around it) and a recent cubeb update in Bug 1341238 uses it and there doesn't seem to be a non-intrusive cutover there. The options seemed to be having libgcc/stdc++ support win32 threads natively or switching to POSIX-based threading in MinGW with pthread emulation. The latter is, as far as I can tell, the only realistic option.
Comment 1•7 years ago
|
||
(In reply to Tom Ritter [:tjr] from comment #0) > The options seemed to be having libgcc/stdc++ support win32 threads natively > or switching to POSIX-based threading in MinGW with pthread emulation. The > latter is, as far as I can tell, the only realistic option. I agree. I was hoping to see this fixed on GCC side for years, but it never happen and I failed to find the time to do that myself. We need to use pthead.
Assignee | ||
Comment 2•7 years ago
|
||
It looks like in Bug 1347866 Angle started using std::future (https://dxr.mozilla.org/mozilla-central/source/gfx/angle/src/libANGLE/WorkerThread.h) - which might not even be usable once we fix the threading problems...
Comment 4•7 years ago
|
||
Hey Tom, I haven't got a window environment to actually look into this bug but after a quick investigation on it, it looks like the recent MinGW already comes with pthread support [1]. Have you already known this but been unable to take it for some reason? [1] http://stackoverflow.com/questions/29947302/meaning-of-options-in-mingw-w64-installer
Comment 5•7 years ago
|
||
Another reference just for notes: http://stackoverflow.com/questions/37358856/does-mingw-w64-support-stdthread-out-of-the-box-when-using-the-win32-threading
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Henry Chang [:hchang] from comment #4) > Hey Tom, > > I haven't got a window environment to actually look into this bug > but after a quick investigation on it, it looks like the recent MinGW > already comes with pthread support [1]. Have you already known this but > been unable to take it for some reason? So this is for a MinGW cross-compile - compiling for Windows on Linux. So it's not a matter of selecting the right option when setting up MinGW. I have most of a script that replicates the MinGW build in Docker right now, but I ran into a couple of errors I hadn't encountered before because I haven't been using Docker recently. Getting solutions (even temporary workarounds) for Bug 1364560 and Bug 1363506 is going to be necessary to enable a complete MinGW build, and those will probably block progress on this bug. In the interim, the type of thing we're looking at is the partial script I've attached. If you complete it, you'll get build errors, but you'll at least understand the environment we're working in.
Assignee | ||
Comment 7•7 years ago
|
||
Okay, this new script will create a complete build toolchain, check out central, and apply the appropriate patches to complete the mingw compilation.
Attachment #8867338 -
Attachment is obsolete: true
Comment 8•7 years ago
|
||
Hi Tom, Sorry i haven't got spare cycles to work on this bug. To help Ethan and me figure out its priority, I am going to summarize what you mentioned before: 1) We can *always* use #ifdef __MINGW32__ to get around the MinGW build failure caused by std::thread (even though it's pretty annoying and sometimes intrusive) so this bug is not supposed to block MinGW build on windows. 2) According to comment 2, we are using more and more std::thread-dependent STL libraries (i.e. future,async) and they might still not be usable even we fix the threading issue. 3) Bug 1364560 is also blocking this bug. Are my summary all right or some are not quite correct? Thanks.
Flags: needinfo?(tom)
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Henry Chang [:hchang] from comment #8) > Hi Tom, > > Sorry i haven't got spare cycles to work on this bug. To help Ethan and me > figure out its priority, I am going to summarize what you mentioned before: > > 1) We can *always* use #ifdef __MINGW32__ to get around the MinGW build > failure > caused by std::thread (even though it's pretty annoying and sometimes > intrusive) > so this bug is not supposed to block MinGW build on windows. We have been able to use __MINGW32__ tricks in Bug 1341238 and Bug 1317642; but there are other uses where it is not so easy. For example https://dxr.mozilla.org/mozilla-central/source/media/libcubeb/src/cubeb_log.cpp#76 Moving into the future, it will not be feasible to comment out all std::thread uses using __MINGW32__. So 'always' is incorrect. > 2) According to comment 2, we are using more and more std::thread-dependent > STL libraries (i.e. future,async) and they might still not be usable even > we fix the threading issue. Once threading is fixed I believe these other STL libraries will work. > 3) Bug 1364560 is also blocking this bug. No. The patch attached to that bug will get a MinGW build working. While that bug isn't fixed yet; it doesn't stop work on this one.
Flags: needinfo?(tom)
Assignee | ||
Comment 11•7 years ago
|
||
This is a quick-fix hack, need to make this correct, but want to attach the patch before I lost it =)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8905098 -
Flags: review?(nfroyd)
Assignee | ||
Updated•7 years ago
|
Attachment #8898860 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8871437 -
Attachment is obsolete: true
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8905098 [details] Bug 1349912 Do not use win32 local includes for libstagefright on MinGW build https://reviewboard.mozilla.org/r/176914/#review181888
Attachment #8905098 -
Flags: review?(nfroyd) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 14•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/78e94797de06 Do not use win32 local includes for libstagefright on MinGW build r=froydnj
Keywords: checkin-needed
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/78e94797de06
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
status-firefox55:
affected → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•