[Box Backup-dev] 0.11rc1 win32 code review

[Chris Wilson]

Hi Charles,

On Sun, 27 Jan 2008, Charles Lecklider wrote:

> I had a few spare minutes today so I thought I'd take a quick look 
> through the code for rc1. I've not looked through everything, but what I 
> did seems to have far fewer bits of Win32-specific stuff scattered 
> through it, and that can only be a good thing.
> 
> If there are any Win32 parts I've not listed that you think are worth a 
> look let me know and I'll try to find some more time later this week.

Many, many thanks for taking the time to do this! Most of the things you 
pointed out look to be relatively easy to fix. I'll address them over the 
next few days.

But I have a big problem with the overlapped I/O stuff. Basically it makes 
the whole code for WinNamedPipeStream very difficult to read and 
understand the correctness of. I get a lot of errors from this code during 
the unit tests on Windows, and it sometimes hangs on XP (perhaps even on 
2K).

I'd really like to remove overlapped I/O completely. But I have a bit of a 
problem with that too, which is why I introduced it in the first place.

On Windows, we use a thread in BackupDaemon to manage the named pipe. 
Because it appears to be unsafe (causes deadlocks) to access the pipe in 
multiple threads (e.g. to Write() status messages from the main thread 
while the listener thread is blocked on Read()), I have to pass messages 
from the main thread to the listener thread and signal an Event when a new 
message is available for the listener to write to the named pipe.

Now the problem comes when multiple messages are received by the named 
pipe (from the control client). I haven't found a way to tell if there are 
still more messages (or bytes) queued on the pipe. If I just 
WaitForMultipleObjects(main-thread-event, named-pipe) then I don't know 
how many messages to read from the named pipe. If I just read one, then I 
could miss an important message, never read it and deadlock with the 
control client. If I read more than one, I'll end up blocking on Read() 
and never read any more messages from the main thread, so I could also 
deadlock if the control client is waiting for a message from the main 
thread (such as sync finished).

I really wanted to do a non-blocking read on the named pipe, to discover 
whether there are more messages waiting without blocking (and probably 
deadlocking) but I can't see how. Do you have any ideas about this?

Second question (easier I hope): I noticed that Win32 has a Timer API 
(CreateTimerQueueTimer) and I tried to use this to replace the horrible 
RunTimer thread, but the events seem to be delivered very late or not at 
all. I can post sample code if it helps (I don't want to commit it because 
it's badly broken), but do you have any recommendations about it or ideas 
why it might not be working properly? I think you recommended using 
something different in the past, was it Waitable Timer objects? Do they 
work better? 

Cheers, Chris.
-- 
_____ __     _
\  __/ / ,__(_)_  | Chris Wilson <0000 at qwirx.com> - Cambs UK |
/ (_/ ,\/ _/ /_ \ | Security/C/C++/Java/Ruby/Perl/SQL Developer |
\ _/_/_/_//_/___/ | We are GNU : free your mind & your software |