Re-examining SetThreadContext

[Mike Hearn]

Hi Alexandre,

A user recently reported another app that doesn't work due to Armadillo
copy protection. This scheme requires SetThreadContext/GetThreadContext to
work properly and currently they do not.

These two functions require the thread to be suspended, therefore they
always return a bogus context (blocking on a wineserver wait pipe).

I wrote a patch for this a while ago:

http://navi.cx/svn/misc/trunk/winekb/patches/thread-context.patch

However you felt it had races in it. There were two possible races I could
see:

- The race between suspending the thread and the sigcontext being uploaded.
  This was not a problem with my patch as Get/SetThreadContext loop.

- A race where you can call SuspendThread again before the SIGUSR1 handler
  has completed. I believe this was the sticking point last time, no? But
  I can't see how this can happen because SIGUSR1 is blocked while it's
  being handled.

So in the following sequence:

PROCESS A: SuspendThread()
PROCESS A: GetThreadContext()
PROCESS B: SIGUSR1
PROCESS B: upload sigcontext
PROCESS B: block
PROCESS A: GetThreadContext() returns
PROCESS A: ResumeThread
PROCESS B: unblock, download sigcontext, restore, finish

I do not see where the races are. Could you please re-examine this patch
and let me know if there are any more problems with it?

Dealing with the set/thread context operations in SIGUSR1 makes sense
because they are only allowed when the target thread is suspended.

thanks -mike

[Alexandre Julliard]

Mike Hearn  writes:
> Dealing with the set/thread context operations in SIGUSR1 makes sense
> because they are only allowed when the target thread is suspended.

What makes you think that?  There is no such restriction in the API,
you can get/set the context without suspending the thread.  The
obvious use case is a thread setting its own context.

[Mike Hearn]

Alexandre Julliard wrote:
> What makes you think that?  There is no such restriction in the API,
> you can get/set the context without suspending the thread.  The
> obvious use case is a thread setting its own context.

Hmm, you must know something I don't :)

MSDN (I know, I know ...) says:

"You cannot get a valid context for a running thread. Use the 
SuspendThread function to suspend the thread before calling 
GetThreadContext."

and

"If you call GetThreadContext for the current thread, the function 
returns successfully; however, the context returned is not valid."

also

"Do not try to set the context for a running thread; the results are 
unpredictable. Use the SuspendThread function to suspend the thread 
before calling SetThreadContext."

So I'd be surprised if any app actually relies on being able to do those 
things, as it's either not allowed or the results are likely to be garbage.

As for a thread setting its own context, I think it'd be OK to check for 
that and do a FIXME for now. That way if an app really tries to do it, 
we can use some custom assembly to achieve the right effect without 
getting the wineserver involved.

thanks -mike

[Alexandre Julliard]

Mike Hearn  writes:
> So I'd be surprised if any app actually relies on being able to do
> those things, as it's either not allowed or the results are likely to
> be garbage.

Well, you'll be surprised. In the general case the results may well be
unpredictable but there are lots of cases where they are perfectly
predictable, and some apps do rely on that.
> As for a thread setting its own context, I think it'd be OK to check
> for that and do a FIXME for now. That way if an app really tries to do
> it, we can use some custom assembly to achieve the right effect
> without getting the wineserver involved.

No need for a FIXME, we already know that some apps do it. Also custom
assembly is not enough, for instance you can't set debug registers
that way.

[Mike Hearn]

Alexandre Julliard wrote:
> Well, you'll be surprised. In the general case the results may well be
> unpredictable but there are lots of cases where they are perfectly
> predictable, and some apps do rely on that.

Ick. OK, how about implementing them using a SuspendThread call, so we 
can use SIGUSR1 for both? Would suspending the thread, altering the 
context, then resuming it in a blocking fashion give close enough 
behaviour to what the apps expect?

thanks -mike

[Alexandre Julliard]

Mike Hearn  writes:
> Ick. OK, how about implementing them using a SuspendThread call, so we
> can use SIGUSR1 for both? Would suspending the thread, altering the
> context, then resuming it in a blocking fashion give close enough
> behaviour to what the apps expect?

This could probably be made to work, yes.