Re: [1/2] server: Never attempt to wake up the current thread.

[Sebastian Lackner]

On 16.12.2014 06:23, Sebastian Lackner wrote:
> Alternative approach for patch #107840 "server: Avoid sending unexpected wakeup with uninitialized cookie value." (which should also still be fine, but this solution is probably a bit easier).
>
> We never want to send any wakeup cookies to our currently active thread, so just fake success in this case.
> To demonstrate what happens without this patch (and to avoid any regressions) I've also added a test in patch 2/2.
>
> The unexpected cookies itself are not visible without debug messages of course, but because of the buffer size limitation the whole wineserver freezes.
> When forcefully killing the test, the wineserver also crashes with:
>
> ==14785== Invalid read of size 4
> ==14785==    at 0x807CE1B: select_on (thread.c:830)
> ==14785==    by 0x807E604: req_select (thread.c:1462)
> ==14785==    by 0x8076003: call_req_handler (request.c:247)
> ==14785==    by 0x807620A: read_request (request.c:302)
> ==14785==    by 0x807B8C1: thread_poll_event (thread.c:267)
> ==14785==    by 0x805679F: fd_poll_event (fd.c:446)
> ==14785==    by 0x8056A43: main_loop_epoll (fd.c:541)
> ==14785==    by 0x8056E2F: main_loop (fd.c:886)
> ==14785==    by 0x805F4B6: main (main.c:148)
> ==14785==  Address 0x50 is not stack'd, malloc'd or (recently) free'd
> ==14785== 
> ==14785== Process terminating with default action of signal 11 (SIGSEGV): dumping core
>
> Reason for the crash is that the wakeup write() syscall fails -> the thread is terminated with kill_thread() -> this sets current = NULL - the code later assumes that 'current' is still a valid pointer. By not sending wakeup cookies we avoid this whole code path, and the test runs well.
>
> ---
>  server/thread.c | 4 ++++
>  1 file changed, 4 insertions(+)

Please ignore this patch for now, there are some situations (not covered by existing tests) where this idea would not work correctly.