Will submit a patch later on this evening... Bill
----- Original Message ----- From: "Bill Stoddard" <[EMAIL PROTECTED]> To: "Cliff Woolley" <[EMAIL PROTECTED]> Cc: <[EMAIL PROTECTED]>; "APR Development List" <[EMAIL PROTECTED]> Sent: Tuesday, March 05, 2002 5:24 PM Subject: Re: apr_file_open() and caching file descriptors > Haven't had much time to think about this today, but I did discover that the XTHREAD > support in win32 apr_file io is seriously broken. apr_file_open(APR_XTHREAD) on >Windows > should -not- be creating the overlapped structure and the io completion event. If an open > file is being shared across thread, each thread should have it's own instance of > apr_file_t with its own instance of the overlapped structure and the completion >event. As > it is now, if two threads both try to read from a file opened for overlapped i/o at >the > same time, thread 1 might get the io completion notification for the io issued by >thread 2 > or visa-versa. Not good. > > Bill > > > On Mon, 4 Mar 2002, Bill Stoddard wrote: > > > > > > apr_file_open(yadda,..., APR_XTHREAD|APR_DO_NOT_REGISTER_CLEANUP, r->pool); > > > > I don't conceptually have a problem with the APR_NO_CLEANUP flag (or > > whatever it would be called), but I do have a problem with using > > apr_os_file_get/apr_os_file_put for this. There has got to be a better > > way. (For one thing, your APR_XTHREAD flag is meaningless if you do > > that.) > > > > > Rather than an option to not register a cleanup, perhaps a function to > > > kill the cleanup would be more generally useful. > > > > > > apr_file_kill_cleanups(apr_file_t *file); > > > > You still have a problem with the apr_file_t disappearing when r->pool > > goes away, meaning you'd still need the apr_os_file_get/put thing, which > > just doesn't seem like a good idea to me. There has got to be a good way > > to do this... I'll keep thinking on it and get back to you asap. > > > > --Cliff > > > > -------------------------------------------------------------- > > Cliff Woolley > > [EMAIL PROTECTED] > > Charlottesville, VA > > > > >
