[Bug 29168] Star Wars: The Old Republic game client hangs at intro splash

wine-bugs at winehq.org wine-bugs at winehq.org
Sat Mar 10 07:43:28 CST 2012


http://bugs.winehq.org/show_bug.cgi?id=29168

--- Comment #145 from Carsten Juttner <carjay at gmx.net> 2012-03-10 07:43:28 CST ---
(In reply to comment #138)
> order; from what I read, it should be done like
> 
>     user_shared_data->SystemTime.High2Time = now.HighPart;
>     user_shared_data->SystemTime.LowPart = now.LowPart;
>     user_shared_data->SystemTime.High1Time = now.HighPart;

You are right, I remember I wanted to scrutinize this a bit more but obviously
failed to do so and ended up copying and pasting the original wine source code
from the initial initialisation.

So let's do this now. We have L, H1 and H2. Obviously on the "kernel" side this
is coherent (since the now-structure is not changing).

Writing L, H1, H2 (as in the patch) concurrently with the expected H1, L, H2
read sequence (in a different thread) leads to 4 cases:

4 relevant points in time:
- H1 old, L old, H2 old: ok
- H1 old, L new, H2 old: nok, but will not(!) be detected
- H1 new, L new, H2 old: nok
- H1 new, L new, H2 new: ok

So indeed this code is obviously wrong and whoever wrote this probably thought
that it would not matter because it was intended as a one time initial write.

Shows that copying and paste code without reviewing is a bad practice. :)

For the intended write order H2, L, H1 and read order H1, L, H2 this cannot
happen (but see below):
- H1 old, L old, H2 old: ok
- H1 old, L old, H2 new: nok
- H1 old, L new, H2 new: nok
- H2 new, L new, H2 new: ok

Since this only becomes an issue at the moment the HighTime-Part actually
changes this could in theory happen every (1<<32)/10e6 = 429 seconds which is
roughly every 7 minutes.

So for a reader application this will appear like the system jumps in time by
about 7 minutes.

Sadly, even if the order is corrected in the patch I think this will not be
enough since due to the hackish nature several updates could happen
concurrently.

Because, if several threads enter a critical section and thus update the page
at the same time this may invalidate the strict order guarantee (since a second
thread may overwrite H2 with an old value after another thread set it to a new
one). So there is still a chance that H and L do not fit.

For a final implementation this also needs to take memory reference reordering
and cache coherence issues into account. IIRC x86 is safe here as long as the
compiler does not reorder the assembler instructions but for other platforms
this may differ.

Some memory fences should help.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
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.



More information about the wine-bugs mailing list