Norbert Unterberg wrote:
> Today I had the chance the experience another explorer lookup and was
> able to attach to the explorer process using VS2003, having the complete
> TSVN debug info present. I found out that the explorer hang at the
> WaitForSingleObject(m_hEvent) command waiting for the pipe transacation
> to complete. The event was never signaled.
>
> I then reviewed the code around the pipe opearions and this is what I
> found out:
>
> #1: The event created for the overlapped opeartion was created as a
> named sync object. That means that multiple instances of explorer.exe
> would share the SAME event object. So when two instances of the explorer
> are talking to the cache simultaneously, one finished transaction might
> wake up both waiting clients, leaving one with wrong data.
Oops. You're absolutely right there!
> #2: The event object was created with bManualReset=FALSE, creating an
> auto-reset event. According to the documentation of overlapped I/O and
> TransactNamedPipe, you should better pass a manual-reset event. However,
> I do not think this causes a problem on it's own, but it can definately
> lead to deadlocks in combination with issue #1.
Again, very right.
> #3: The case where TransactNamedPipe returns TRUE was not handled
> correctly. Even if it is not very likely, overlapped opeartions *can* to
> my knowledge complete immediatley, perhaps(?) leaving the overlapped
> structure and its event object untouched (I did not find a proove of
> this in the docs, so we should be prepared for either behaviour).
> WaitForSingleObject was still called in this case, possibly waiting
> forever for a transaction that might already be finished.
I agree here to handle all possibilities.
> #4: The pipe server in TSVNCache calls WaitNamePipe(). Though I do not
> believe it causes any harm there, it is normally used on the client side
> to wait for the next free instance of a pipe before connecting to the
> pipe. It should be called if the OpenFile errors out with a
> ERROR_PIPE_BUSY error.
I added that call to have a timeout. Without a timeout, the
ConnectNamedPipe() call will wait infinitely until a client connects,
leaving the thread in a waiting state. When closing the application,
that thread won't exit cleanly.
But I guess we can work around that by letting our WM_CLOSE handler do a
dummy connect to that thread (which it already does;) ).
> I created a patch that rearranged the pipe calls to clean up these
> issues. I am just rebuilding all, so you should not commit it until
> tested. In the meantime please feel free to comment.
The patch looks very good. I'm currently doing some testing with it.
As soon as you give the 'go' signal, I'll commit it.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Sat Apr 30 13:15:06 2005