[Bug 46099] New: Star Citizen not loading after implementation of WaitOnAddress() in wine
https://bugs.winehq.org/show_bug.cgi?id=46099 Bug ID: 46099 Summary: Star Citizen not loading after implementation of WaitOnAddress() in wine Product: Wine Version: 3.19 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown Assignee: wine-bugs(a)winehq.org Reporter: rawfox(a)freenet.de Distribution: --- Since commit 25acfbb40b5316a8c87989d4e61ce9edef2a04aa introduced RtlWaitOnAddress(), the Star Citizen game gets stuck loading early. https://github.com/wine-mirror/wine/commit/25acfbb40b5316a8c87989d4e61ce9ede... Stubbing out the function makes the game load again under all following wine commits up to current. Here is a patch to make the game loading again https://pastebin.com/9zNBB83T Logfile (+timestamp,+tid,+kernelbase) of a fresh compiled and unmodified wine-3.19 (vanilla): https://pastebin.com/es6sr6Dg The launcher loads ok, starting the game, but its not opening the very first loading window, gets just stuck. Same logfile after stub out WaitOnAddress(): https://pastebin.com/e3797VAA Ive cut the logfile, it goes on for some time with the tons of stubbed RtlCaptureStackBackTrace but the game was loading, is playable and quitable as well. Commit 25acfbb40b5316a8c87989d4e61ce9edef2a04aa broke that ability and needs this patch to prevent the loading get stuck. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #1 from rawfox <rawfox(a)freenet.de> --- Here is the related diskussion on WineHQ forums: https://forum.winehq.org/viewtopic.php?f=2&t=31350&p=119007#p119007 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression Component|-unknown |kernelbase Regression SHA1| |7c430f5b3cef93ac3ce85278ad9 | |253afc81cfc40 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Robert Walker <bob.mt.wya(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bob.mt.wya(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Robert Walker <bob.mt.wya(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bob.mt.wya(a)gmail.com --- Comment #2 from rawfox <rawfox(a)freenet.de> --- Created attachment 62727 --> https://bugs.winehq.org/attachment.cgi?id=62727 wine-3.19 (vanilla) Log of starting SC getting stuck WINEDEBUG=+timestamp,+tid,+kernelbase -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #3 from rawfox <rawfox(a)freenet.de> --- Created attachment 62728 --> https://bugs.winehq.org/attachment.cgi?id=62728 wine-3.19 (vanilla) patched SC starting to menu screen then i quit the game, exits normal. Full og with WINEDEBUG=+timestamp,+tid,+kernelbase -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #4 from rawfox <rawfox(a)freenet.de> --- Created attachment 62729 --> https://bugs.winehq.org/attachment.cgi?id=62729 StubWaitOnAddress This patch works on all new commits for now but aint fixing the root cause of the game getting stuck on load. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 rawfox <rawfox(a)freenet.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rawfox(a)freenet.de -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Anastasius Focht <focht(a)gmx.net> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=46047 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Tool <codedonewell(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |codedonewell(a)gmail.com --- Comment #5 from Tool <codedonewell(a)gmail.com> --- Created attachment 62731 --> https://bugs.winehq.org/attachment.cgi?id=62731 wine-3.19-staging Thread Log For RtlWaitOnAddress Hang Ref comments in bug: lines 298-303 show the use of WakeAddressAll The hang itself is at the end of the file. I used an awk script to split the main debug log into per threads logs, so I could follow what is happening. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #6 from Tool <codedonewell(a)gmail.com> --- Created attachment 62732 --> https://bugs.winehq.org/attachment.cgi?id=62732 wine 3.18-staging thread log matching the 3.19 one previously attached Ref bug comments: Lines 308-311 show RtlWakeAllConditionVariable being used in 3.18 (I expected a stub message for the WakeOnAddress version) Log truncated as it is big and never hangs -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #7 from Tool <codedonewell(a)gmail.com> --- I have been trying to explore the problem - hoping that I might even be able to contribute here. My last approach was to compare the working log for a thread in 3.18 to the log for the same thread that does hang in 3.19. I have attached thread logs for the same thread at the same phase of loading Star Citizen. I have identified these threads by following what threads are created by the StarCitizen.exe launch process. These logs are for the thread that is launched for the same purpose. By following the various activities (exception traces, SRWLock activity, Virtual memory activity) I believe I have found a different method being used in each version to achieve the same goal. The 3.19 version shows the hang at the end, however, the first bit that I need help with is that I cannot figure out: - why there were no WaitOnAddress "stub" messages in the 3.18 log; and - how the different function was selected in each version. I have 20+ years experience with C (OK a fair chunk of Java in there), but only about a year of that with Win32. I am totally willing to keep trying things to try to find the problem, but I'm not sure where to take it - and my confidence is somewhat dented by not being able to figure out the above 2 items. If anyone has theories that they would like me to test - I'll do it! Need different logs? Done! (Warning the full log in the hang case is 1.4GB uncompressed) My tests were conducted with WINEDEBUG=fixme+all,trace+all,trace-heap,+relay I can provde my RelayExcludes if asked - they are big. The main log was split into thread logs with an awk script (also available on request), and some extra relay messages have been filtered by that. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #8 from Tool <codedonewell(a)gmail.com> --- Ooops, sorry forgot to mention what the two different methods were: wine 3.19 uses RtlWakeAddressAll (lines 298-303) wine 3.18 uses RtlWakeAllConditionVariable (lines 308-311) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #9 from Tool <codedonewell(a)gmail.com> --- Moving on from my ignorance above and looking further at the 2 log sets that I have, I believe that I should be able to identify where the problem occurs. The nature of the symptoms are consistent with a "missed signal" so I'll obviously be looking for that lack of required synchronisation first. In my experience, deadlocks are easy to misdiagnose so I need to be excruciatingly methodical. It will be several days before I have the time to give it the necessary hours of attention though. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dark.shadow4(a)web.de --- Comment #10 from Fabian Maurer <dark.shadow4(a)web.de> --- Is there a free demo one could test with? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #11 from Tool <codedonewell(a)gmail.com> --- Sadly there is no free demo. Even then, there are more fundamental issues with the install process under wine, so you have to actually install it under windows, and then copy the windows installation directory into a wine prefix. Although it is probably technically an EULA violation, I do have a second account that I think I can handover to you to use. I would expect it back eventually though ;) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #12 from Fabian Maurer <dark.shadow4(a)web.de> --- Appreciate your offer, but wouldn't want to risk you getting into problems. Probably wouldn't be able to fix it, just like to look into complex stuff. And sometimes it works out. ;) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 George Gibbs <vash63(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |vash63(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 rawfox <rawfox(a)freenet.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Distribution|--- |Fedora -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Jason <holzminister(a)t-online.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |holzminister(a)t-online.de --- Comment #13 from Jason <holzminister(a)t-online.de> --- Star Citizen Alpha will be free to access from November 23 to November 30, if someone without an payed account would like to investigate this. Source: https://www.pcgamer.com/star-citizen-is-going-free-for-a-week-and-you-get-a-... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #14 from Greg Smith <codedonewell(a)gmail.com> --- Created attachment 62807 --> https://bugs.winehq.org/attachment.cgi?id=62807 Bug shown here. Waiting thread not notified. This is a snippet of the wine debug output. It clearly shows that thread 009e waits on address 02c7cf00, and thread 0090 subsequently calls RtlWakeAddressAll on 02c7cf00, but 009e is never sent a *wakeup*. The only mention of thread 009e after this is when it is killed by my <ctrl>+c when I gave up waiting for the app to continue. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #15 from Greg Smith <codedonewell(a)gmail.com> --- Diving a bit deeper: When thread 009e calls WaitOnAddress/RtlWaitOnAddress, it does some necessary checks before actually waiting, the problem is occuring because after it has performed the checks but before it can get a lock on keyed_event, thread 0090 has called RtlWakeAddressAll and changed that address' contents. When thread 009e goes into the wait code, the address has been changed but it never gets notified. Had thread 0090 been forced to wait until 009e had completed it checks and started it's wait, then the deadlock would not have happened. I'm not yet sure of a good way to provide the necessary synchronisation here. I'll have to research how it is done elsewhere in wine. I suspect that attention from more experienced wine devs is called for. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #16 from Fabian Maurer <dark.shadow4(a)web.de> ---
the problem is occuring because after it has performed the checks but before it can get a lock on keyed_event, thread 0090 has called RtlWakeAddressAll and changed that address' contents. When thread 009e goes into the wait code, the address has been changed but it never gets notified.
This would make sense. AFAIK this can't happen on windows. Though I wouldn't know how I'd write a test for that, to be honest. If that's not done yet, we'd need to buffer the call from WakeAddress, so that when going to sleep we can skip right over it (when the value is actually changed). Only keeping the most recent "wake" in a buffer should be fine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #17 from Greg Smith <codedonewell(a)gmail.com> --- Created attachment 62875 --> https://bugs.winehq.org/attachment.cgi?id=62875 Test to highlight the bug This test starts a group of "load" threads (CPU count * 4) whose purpose is only to burden the CPU - theoretically making the crucial race condition more likely. It then starts 2 threads that wait and wake (all) on the same address. The only difference is that the second thread skips the first wait. The test fails if any wait takes longer than 10 seconds to be notified. Purer versions of this test mean bigger wait times. The failure occurs quite quickly on my 2 core 4 thread processor, although if you have a high thread to core ratio, the test may not fail due to insufficient load. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #18 from Greg Smith <codedonewell(a)gmail.com> --- Upon review, I'm not 100% sure that the test won't provide a false positive under load :( It doesn't pass with a crosstest on win10 sadly, so it proves nothing. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #19 from Zebediah Figura <z.figura12(a)gmail.com> --- The way it'd normally be handled is in the kernel. We need to be able to very that, at the exact point that the thread is queued on select(), the value still matches the comparand. I think this can be done, but it may require some tweaks to wineserver select interface. I'll try to write a patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #20 from Fabian Maurer <dark.shadow4(a)web.de> ---
We need to be able to very that, at the exact point that the thread is queued on select(), the value still matches the comparand.
What do you mean? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #21 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #20)
We need to be able to very that, at the exact point that the thread is queued on select(), the value still matches the comparand.
What do you mean?
If we check that the value matches before queueing, we can get a race of the form: thread A: checks value, still equal to comparand thread B: sets value thread B: perform wake thread A: queue self We need to prevent such a wake from occurring between the time thread A checks the value for the last time, and the time it queues itself. The Linux kernel manages this with futexes by locking the queue and then (effectively) atomically comparing the value and queueing the thread. We can't really use this approach, since we can't be reading process memory from the wineserver. My idea was to split queueing and waiting into two separate operations for futexes, though now I'm wondering if perhaps a CS might be better here. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #22 from Greg Smith <codedonewell(a)gmail.com> --- (In reply to Zebediah Figura from comment #21)
(In reply to Fabian Maurer from comment #20)
We need to be able to very that, at the exact point that the thread is queued on select(), the value still matches the comparand.
What do you mean?
If we check that the value matches before queueing, we can get a race of the form:
thread A: checks value, still equal to comparand thread B: sets value thread B: perform wake thread A: queue self
We need to prevent such a wake from occurring between the time thread A checks the value for the last time, and the time it queues itself.
The Linux kernel manages this with futexes by locking the queue and then (effectively) atomically comparing the value and queueing the thread. We can't really use this approach, since we can't be reading process memory from the wineserver.
My idea was to split queueing and waiting into two separate operations for futexes, though now I'm wondering if perhaps a CS might be better here.
Thanks for looking at this. I am just too new to wine changes to be confident in the impacts of making the changes that I think are required. Looking at what is there, I failed to think of a way to do it without changing or at least copying the wineserver_select code - something that my experience tells me is almost always means I don't understand how it works well enough. I also think that CS may be a better path, but there seems to always be an opportunity for a race condition. I certainly have an even greater respect now for what pthread_cond_wait achieves! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #23 from Fabian Maurer <dark.shadow4(a)web.de> ---
I also think that CS may be a better path, but there seems to always be an opportunity for a race condition
Should work without a race condition though. Spurious wakes is okay, freezes are not.
We can't really use this approach, since we can't be reading process memory from the wineserver.
Why is that? Don't we have to do comparing and queuing atomically? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #24 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #23)
We can't really use this approach, since we can't be reading process memory from the wineserver.
Why is that? Don't we have to do comparing and queuing atomically?
Because we can't rely on being able to read process memory from the wineserver. It's a feature that should only be used for debugging. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #25 from Fabian Maurer <dark.shadow4(a)web.de> ---
Because we can't rely on being able to read process memory from the wineserver. It's a feature that should only be used for debugging.
I currently don't see a way it would work without. Mind explaining your alternative? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #26 from Zebediah Figura <z.figura12(a)gmail.com> --- Created attachment 62895 --> https://bugs.winehq.org/attachment.cgi?id=62895 patch set to fix race I've attached a set of two patches that should hopefully avoid the race discussed above. Can you please test this with Star Citizen and see if it fixes the bug? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #27 from Dmitry Timoshkov <dmitry(a)baikal.ru> --- (In reply to Zebediah Figura from comment #26)
Created attachment 62895 [details] patch set to fix race
Is that intentional that compare_addr() performs the comparison differently for different sizes? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #28 from Zebediah Figura <z.figura12(a)gmail.com> --- Created attachment 62896 --> https://bugs.winehq.org/attachment.cgi?id=62896 patch set to fix race (v2) (In reply to Dmitry Timoshkov from comment #27)
(In reply to Zebediah Figura from comment #26)
Created attachment 62895 [details] patch set to fix race
Is that intentional that compare_addr() performs the comparison differently for different sizes?
Whoops, thanks for the catch. Please try this instead. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62895|0 |1 is obsolete| | -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #29 from Fabian Maurer <dark.shadow4(a)web.de> ---
I've attached a set of two patches that should hopefully avoid the race discussed above. Can you please test this with Star Citizen and see if it fixes the bug?
Ah thanks, that makes sense. Guessed on a race condition between queuing and going to sleep, but it's differently implemented, so nevermind. FWIW, your patches fix all the race conditions I was able to reproduce. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #30 from Greg Smith <codedonewell(a)gmail.com> --- That v2 patch works wonderfully for me. Thanks heaps Zebediah. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #31 from rawfox <rawfox(a)freenet.de> --- The v2 patch works with wine-3.21 (vanilla) The game does load up again, thats just wonderful <3 Now, the wine-staging patches wont apply anymore but from what i understood, its a fundamental issue in the wine kernel on thread races. With that, those mechanics would need an implementation into wine itself, not staged out into the staging branch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 minion <minion.procyk(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |minion.procyk(a)gmail.com --- Comment #32 from minion <minion.procyk(a)gmail.com> --- I tried the patch and it does prevent my star citizen from crashing (which is awesome), however I'm still unable to actually load the game. Glad to hear you got it working though =] I tried doing a wine-staging build and then applying the patch, then tried a vanilla build with the patch and got the same results with and without dxvk. Nothing crashes, it just sits at the splash screen. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #33 from Greg Smith <codedonewell(a)gmail.com> --- I'm sorry to hear that you didn't get in. Both rawfox and I do use dxvk, so I am not sure if there are currently issues without using it. I did get in without dxvk a few weeks back, but there were clear visual issues with textures not loading properly. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #34 from Zebediah Figura <z.figura12(a)gmail.com> --- Created attachment 62897 --> https://bugs.winehq.org/attachment.cgi?id=62897 ntdll: Add a futex-based implementation of WaitOnAddress(). Thanks for the confirmation. I have another patch here, which implements WaitOnAddress() on top of futexes. It's a bit of an unorthodox approach, but is the best I was able to come up with. I'd appreciate it if people could test it (applied on top of the other patches in this thread) with a real application. Fabian, this patch fails your tests, because it treats WakeByAddressSingle() like WakeByAddressΑll() and wakes up every thread. Since spurious wakeups should always be anticipated by the application, I doubt this should break anything, but since you wrote the tests, are you aware of any application that depends on that behaviour? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #35 from Greg Smith <codedonewell(a)gmail.com> --- I was not able to get into the game with that last patch applied. In my first attempt, it got very close (2nd stage loading screen), but hung. In my second attempt it did not make it past the first stage loading screen, but it did populate it. In my third attempt, it did not populate the first stage loading screen. My fourth attempt was the same as the second attempt. An interesting observation: in all but the first attempt, 1/4 of the overall CPU was being used (100% showed by top) - however the load was distributed across all 4 cores - so maybe some sort of no-change-spin issue? I only tested the previous patches once, so I'll go back and give it another go with just those applied, to make sure that I get consistent behaviour -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #36 from Greg Smith <codedonewell(a)gmail.com> --- The retest using v2 patches got me into the game 4 out of 5 times, and the one time that it didn't may have been a StarCitizen issue, as the server in the previous connection was displaying performance issues (NPC's stuck, doors not auto opening), where as the server in the subsequent success attempt had one of those issues. So at this stage, I'd say that the v2 patches are good, but the futex patch is not. It seems the original problem does always occur as soon with the futex patch - I'm not sure if that helps. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #37 from rawfox <rawfox(a)freenet.de> --- The v2 patch worked but caused a huge loss of game performance, drops 80%. The futex patch applied on top of v2 made the game load but its crashing with a freeze in less then 2 minutes, sometimes make it into the game with now normal fps, sometimes its crashing already before the menu screen comes up. wine-3.21 vanilla -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #38 from Zebediah Figura <z.figura12(a)gmail.com> --- Created attachment 62902 --> https://bugs.winehq.org/attachment.cgi?id=62902 fix for futex-based implementation Right, thanks. Can you please try this additional patch on top of the futex patch? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #39 from Fabian Maurer <dark.shadow4(a)web.de> --- If that doesn't help, just to make sure: Which Linux kernel are you running? There used to be a bug with futex_wait back then. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #40 from rawfox <rawfox(a)freenet.de> --- On a first test i had a normal 15mins session in StarCitizen with the futex fix on top of the others :) uname -a: Linux operator 4.19.4-300.fc29.x86_64 #1 SMP Fri Nov 23 13:03:11 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux We had an update on Fedora glibc and kernel 4.19 is everything else then good but it has been working, used the latest wine commit, will test harder on this and hope i can break it :) Thanks A LOT for your support people \m/ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #41 from Greg Smith <codedonewell(a)gmail.com> --- That patch fix (v4 patch?) works well for me. The performance was about right too - about 5%-10% below what I get on window with the same config (I also had youtube playing a video on a separate screen as well ;) ). Which is consistent with how it worked with wine-staging-3.18. So I'm calling it a win! Thanks again for you help here Zebediah! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #42 from rawfox <rawfox(a)freenet.de> --- ya, after testing this we can consider this stable i think :) Zeb can you tweak the patches, so they apply as well on a wine-staging patched source ? wine-staging touches dlls/ntdll/sync.c as well as the patches do so the patches only go in on a vanilla wine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #43 from Jason <holzminister(a)t-online.de> --- I tested with all of the latest patches and loading up the game works just fine. I also played for about an hour without an crash. (Loreville does not work, but that is not related to this problem I guess) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 rawfox <rawfox(a)freenet.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #44 from rawfox <rawfox(a)freenet.de> --- Ive changed the status to resolved now. This bug is fixed by the patches provided by Zebediah Figura. Ive heared the patches went to wine-staging and need to get approved. There have been at least 10 people involved in fixing this by testing, discussing and spending their time to get this solved. Im sure, other games will benefit from this tweaks as well. Thank you all very mutch for contributing your time into the wonderful wine project :) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |UNCONFIRMED Resolution|FIXED |--- --- Comment #45 from Zebediah Figura <z.figura12(a)gmail.com> --- The patch hasn't been accepted into Wine yet. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Fixed by SHA1| |6060d2703c2e1382f076ad40ea4 | |3980781eaca2c Status|UNCONFIRMED |RESOLVED --- Comment #46 from Zebediah Figura <z.figura12(a)gmail.com> --- Should be fixed by https://source.winehq.org/git/wine.git/commitdiff/6060d2703c2e1382f076ad40ea.... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #47 from rawfox <rawfox(a)freenet.de> --- Boah thats great, can thank you people enuff for doing this !! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #48 from rawfox <rawfox(a)freenet.de> --- Looks like only the v2 patches got approved. The other 2 futex fixes are not in, resulting in bad game performance. They can be applied to the latest wine commit. What now ? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #49 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to rawfox from comment #48)
Looks like only the v2 patches got approved. The other 2 futex fixes are not in, resulting in bad game performance. They can be applied to the latest wine commit. What now ?
These have been submitted: https://source.winehq.org/patches/data/155645 If you'd like to open a new bug report for the performance of SC and to track that patch, please feel free. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #50 from rawfox <rawfox(a)freenet.de> --- (In reply to Zebediah Figura from comment #49)
(In reply to rawfox from comment #48)
Looks like only the v2 patches got approved. The other 2 futex fixes are not in, resulting in bad game performance. They can be applied to the latest wine commit. What now ?
These have been submitted: https://source.winehq.org/patches/data/155645
If you'd like to open a new bug report for the performance of SC and to track that patch, please feel free.
Thats over my horizon, sorry, but it looks different to this https://bugs.winehq.org/attachment.cgi?id=62897 and this https://bugs.winehq.org/attachment.cgi?id=62902 1st was val = *futex; 2nd was val = __atomic_load_n(futex, __ATOMIC_SEQ_CST); <- this is working good 3rd now val = interlocked_cmpxchg( futex, 0, 0 ); In wine-staging you added a futex condition patch what touches sync.c . I removed that locally here and added the two missing patches from above that are not yet in wine. That way i can patch it all together with wine-staging. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #51 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to rawfox from comment #50)
(In reply to Zebediah Figura from comment #49)
(In reply to rawfox from comment #48)
Looks like only the v2 patches got approved. The other 2 futex fixes are not in, resulting in bad game performance. They can be applied to the latest wine commit. What now ?
These have been submitted: https://source.winehq.org/patches/data/155645
If you'd like to open a new bug report for the performance of SC and to track that patch, please feel free.
Thats over my horizon, sorry, but it looks different to this https://bugs.winehq.org/attachment.cgi?id=62897 and this https://bugs.winehq.org/attachment.cgi?id=62902
1st was val = *futex; 2nd was val = __atomic_load_n(futex, __ATOMIC_SEQ_CST); <- this is working good 3rd now val = interlocked_cmpxchg( futex, 0, 0 );
Right; __atomic_load_n() isn't strictly portable; I just used it as a quick proof-of-concept (since all modern compilers should support it). interlocked_cmpxchg( futex, 0, 0 ) is portable and should give the same guarantees wrt. memory ordering.
In wine-staging you added a futex condition patch what touches sync.c . I removed that locally here and added the two missing patches from above that are not yet in wine. That way i can patch it all together with wine-staging.
That's for a different function; see bug 46208 and bug 45524. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 --- Comment #52 from rawfox <rawfox(a)freenet.de> --- ah i see(In reply to Zebediah Figura from comment #51)
(In reply to rawfox from comment #50)
(In reply to Zebediah Figura from comment #49)
(In reply to rawfox from comment #48)
Looks like only the v2 patches got approved. The other 2 futex fixes are not in, resulting in bad game performance. They can be applied to the latest wine commit. What now ?
These have been submitted: https://source.winehq.org/patches/data/155645
If you'd like to open a new bug report for the performance of SC and to track that patch, please feel free.
Thats over my horizon, sorry, but it looks different to this https://bugs.winehq.org/attachment.cgi?id=62897 and this https://bugs.winehq.org/attachment.cgi?id=62902
1st was val = *futex; 2nd was val = __atomic_load_n(futex, __ATOMIC_SEQ_CST); <- this is working good 3rd now val = interlocked_cmpxchg( futex, 0, 0 );
Right; __atomic_load_n() isn't strictly portable; I just used it as a quick proof-of-concept (since all modern compilers should support it). interlocked_cmpxchg( futex, 0, 0 ) is portable and should give the same guarantees wrt. memory ordering.
In wine-staging you added a futex condition patch what touches sync.c . I removed that locally here and added the two missing patches from above that are not yet in wine. That way i can patch it all together with wine-staging.
That's for a different function; see bug 46208 and bug 45524.
Ah ok, i see. Well, when the 2 missing patches are in wine-staging, instead of the futex-condition one you wrote for those bugs, then the game works wonderful while it performes terrible on a vanilla wine now. Can you bring the https://bugs.winehq.org/attachment.cgi?id=62897 and https://bugs.winehq.org/attachment.cgi?id=62902 patches together into that futex condition patch currently in wine-staging what you did for the bugs you mentioned ? That would solve everything ^^ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 rawfox <rawfox(a)freenet.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=46252 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #53 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 4.0-rc1. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=46099 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|kernelbase |kernel32 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla