Ticket #242 (closed defect: fixed)

Opened 10 months ago

Last modified 10 months ago

replicate1 fails on windows

Reported by: charlie Owned by: newbugs
Priority: normal Milestone:
Component: Backend-Remote Version: 1.0.5
Severity: normal Keywords:
Cc: Blocked By:
Operating System: Microsoft Windows Blocking:

Description

In remoteconnection.cc we currently use a pair of file descriptors, which may refer to a socket, stdin/stdout or a file. However we also need in the Windows case to use ReadFile/WriteFile? which require a HANDLE. We use _get_osfhandle to attempt to translate a file descriptor into a HANDLE, however this is not recommended by Microsoft and may cause problems (we have already found that file pointers for files - not sockets or stdin/stdout - are not advanced automatically, meaning we have to manually use an OVERLAPPED structure to keep file pointers correct). Perhaps remoteconnection.cc should open/close files internally, so we could use Windows HANDLES when we need them, or fd's when we don't. Ref: http://support.microsoft.com/kb/99173

Attachments

patch7.patch (3.8 kB) - added by charlie 10 months ago.
Patch to make replicate1 test pass on Windows

Change History

Changed 10 months ago by olly

AIUI, we use the overlapped stuff to implement a non-blocking read with a timeout, not to "keep file pointers correct" (in fact, the only reason we dabble with HANDLE and ReadFile? is so we can implement a non-blocking read with a timeout). Where in the code do you think we're fixing up file pointers?

The current code appears to work. I'm not terribly sympathetic to spending time and effort working around deficiencies in MS APIs which we've already worked around.

Changed 10 months ago by charlie

Patch to make replicate1 test pass on Windows

Changed 10 months ago by olly

  • summary changed from Mixing Windows API calls may cause problems to replicate1 fails on windows

Setting the summary to something helpful

Changed 10 months ago by richard

  • status changed from new to closed
  • resolution set to fixed

I've now applied this patch (with various tweaks), so (if I've done it correctly), we've now worked around the known problems resulting from use of mixed file handling techniques.

However, I'm aware that the replication stuff exposes (for the first time in the Xapian API) a file descriptor. Therefore, when this becomes part of the supported API, we're going to be stuck with using these workarounds. I'm not sure whether there is a cleaner solution, but if there is, we should find it and ensure that the API allows it to be used, before releasing 1.1.

Changed 10 months ago by trac

  • platform set to Microsoft Windows

Changed 10 months ago by olly

Just for the record, note that the MSDN docs for WriteFile?() actually document that the offset in the OVERLAPPED structure isn't updated. So that's not a bug.

Exposing an fd doesn't seem totally ideal if it can be cleanly avoided, but it's certainly possible to be too abstract about such things. I'll have a look and see if anything occurs to me.

We could sidestep using _get_osfhandle() if we implemented this using read()/write() and some sort of alarm which interrupted the read after the timeout. I'm not sure if there's a suitable API for that. One thought is to start a new thread which sleeps for the timeout and then sends a signal, but the thread start overhead is probably excessive for the job. Keeping a special thread about for the job is probably possible.

Note: See TracTickets for help on using tickets.