When I originally read the code for FileStream, I saw the internal _overlapped member and assumed it was part of FileStream when in fact it is part of AsyncFileStream_AsyncResult. BeginWriteCore correctly allocates a new overlapped struct for each write.
Upon closer inspection BeginWrite calls BeginWriteCore (same with BeginRead/BeginWriteCore) that eventually fills in OffsetLow and OffsetHigh of the NativeOverlapped structure (maps to Offset and OffsetHigh of the OVERLAPPED structure in the Platform SDK). It then seems to superfluously call SeekCore(). So, it's already using OVERLAPPED in the way you wanted. That call to SeekCore won't effect where overlapped writes occur. If you're only doing asynchronous IO, FileStream may actually be thread-safe. You could try taking the FileStream code and created a new class that doesn't call SeekCore after setting OffsetLow and OffsetHigh to see if it can be made thread-safe. On Mon, 19 Dec 2005 13:54:47 -0600, John Davis <[EMAIL PROTECTED]> wrote: >I'd have to disagree. If the Seek logic was removed, BeginWriteCore would >be fine for multithreaded access. > > >>From: Peter Ritchie <[EMAIL PROTECTED]> >>Reply-To: "Discussion of advanced .NET topics." >><[email protected]> >>To: [email protected] >>Subject: Re: [ADVANCED-DOTNET] Possible defect in BeginWrite on FileStream >>Date: Mon, 19 Dec 2005 13:42:25 -0500 >> >>It would if you tried to call BeginWrite() concurrently (i.e. at the same >>time, with the same object, on more than one thread). >> >>That was the point of the responses. FileStream was never designed to be >>thread-safe asynchronously or synchronously; meaning concurrent BeginWrite >>calls on the same FileStream object are not supported. Asynchronous IO >>was always supported; but concurrent/multi-threaded asynchronous IO is not. >> >>Seek/BeginWrite is technically atomic in FileStream currently, because you >>can't use it concurrently/multi-threaded--there are no race conditions in >>a single thread and there, and one only file pointer is needed for a >>single IO operation. >> >>The assumption was that you wanted FileStream to use the offset member of >>an OVERLAPPED structure because you wanted concurrent/multi-threaded >>asynchronous IO. If you don't want concurrent asynchronous IO (just >>synchronous IO), then Seek/BeginWrite is an atomic asynchronous >>operation. The side effect of designing a Stream-based class that >>supports concurrent/thread-safe asynchronous IO is it has to use one >>OVERLAPPED structure per IO operation; which the existing FileStream >>implementation is not designed to do. >> >>http://www.peterRitchie.com/ >> >>On Mon, 19 Dec 2005 12:11:20 -0600, John Davis <[EMAIL PROTECTED]> wrote: >> >> >Yes ... is it doing that? >> > >> > >> >>From: Peter Ritchie <[EMAIL PROTECTED]> >> >>Reply-To: "Discussion of advanced .NET topics." >> >><[email protected]> >> >>To: [email protected] >> >>Subject: Re: [ADVANCED-DOTNET] Possible defect in BeginWrite on >>FileStream >> >>Date: Mon, 19 Dec 2005 12:44:11 -0500 >> >> >> >>Of course. But you can't use the same overlapped structure for >>concurrent >> >>writes. >> >> >> >>On Mon, 19 Dec 2005 11:59:07 -0600, John Davis <[EMAIL PROTECTED]> >>wrote: >> >> >> >> >See BeginWriteCore ... it already allocates the OVERLAPPED. >> >> > >> >> >>From: Peter Ritchie <[EMAIL PROTECTED]> >> >> >>Reply-To: "Discussion of advanced .NET topics." >> >> >><[email protected]> >> >> >>To: [email protected] >> >> >>Subject: Re: [ADVANCED-DOTNET] Possible defect in BeginWrite on >> >>FileStream >> >> >>Date: Mon, 19 Dec 2005 11:20:49 -0500 >> >> >> >> >> >>If you want it to be thread-safe (which seems to be the intention of >> >> >>wanting an atomic BeginWrite that accepts offset and data to be any >> >> >>different than calling Seek/BeginWrite), it's quite a bit more >> >> >>complicated. Each call to BeginWrite would have to allocate a new >> >> >>OVERALAPPED structure to atomically track that individual write, and >> >> >>somehow store it in the IAsyncResult object it returns. >> >> >> >> >> >>You'd probably also want the event handle for each of those writes to >>be >> >> >>monitored on individual threads so when it calls the application's >> >> >>callback it doesn't block the completion of any other writes. Again, >> >> >>because we want to add more than Seek/BeginWrite already offers. >> >> >> >> >> >>Nothing ground-breaking to implementing this; but, not the "pretty >>easy" >> >> >>you seem to think. >> >> >> >> >> >>BTW, FileStream.BeginWrite is already asynchronous; "AsyncFileStream" >> >> >>doesn't make sense. >> >> >> >> >> >>http://www.peterRitchie.com/ >> >> >> >> >> >>On Mon, 19 Dec 2005 09:09:52 -0600, John Davis <[EMAIL PROTECTED]> >> >>wrote: >> >> >> >> >> >> >Fair enough, we need an AsyncFileStream class which inherits from >> >> >>FileStream >> >> >> >and has BeginWrite/Read methods on it which contain file offset >> >> >>arguments. >> >> >> >This should be pretty easy since all the async code in there is >> >>already >> >> >> >running off of OVERLAPPED structs. >> >> >> > >> >> >> > >> >> >> >>From: Ian Griffiths <[EMAIL PROTECTED]> >> >> >> >>Reply-To: "Discussion of advanced .NET topics." >> >> >> >><[email protected]> >> >> >> >>To: [email protected] >> >> >> >>Subject: Re: [ADVANCED-DOTNET] Possible defect in BeginWrite on >> >> >>FileStream >> >> >> >>Date: Sat, 17 Dec 2005 14:00:57 -0000 >> >> >> >> >> >> >> >>It seems to me that there's a clear difference between what Seek >> >>gives >> >> >> >>you and what you're asking for. (Otherwise you wouldn't be asking >>for >> >> >> >>it.) I admit that I chose poor terminology to describe the >>feature: I >> >> >> >>should have said "concurrent random access" rather than >>just "random >> >> >> >>access". But I stand by my assertion that it shouldn't be in >>Stream. >> >> >> >> >> >> >> >>The problem I have with adding this to the Stream abstraction is >>that >> >> >> >>all of a sudden, *every* Stream has to support it. >> >> >> >> >> >> >> >>So you undoubtedly *could* have something not dissimilar to Stream >> >>that >> >> >> >>support concurrent random access. But if you made this a feature >>of >> >> >> >>Stream you just made it mandatory for every implementation of >>Stream >> >>to >> >> >> >>support this. >> >> >> >> >> >> >> >>That makes it a non-starter for where we are today - this >>suggestion >> >> >> >>will never fly because Stream shipped almost 4 years ago, and >>there's >> >>no >> >> >> >>way to go and add this functionality retrospectively to all the >> >>Streams >> >> >> >>everyone has implemented ever since. >> >> >> >> >> >> >> >>Given where we are, the only way to add this functionality would >>be >> >>to >> >> >> >>introduce a new more specialized abstraction. (Either specifically >>to >> >> >> >>files, or possibly as some new AsyncStream abstraction.) >> >> >> >> >> >> >> >>So this leaves this discussion as an academic question: should >>Stream >> >> >> >>have had this functionality from the start? >> >> >> >> >> >> >> >>I think Stream's better without this, because this extra feature >> >>would >> >> >> >>increase the cost of implementing Stream. And this is the >>tradeoff >> >>you >> >> >> >>often get with abstractions: making them rich enough to be useful >>but >> >> >> >>simple enough that anyone can be bothered to implement them. >> >> >> >> >> >> >> >>Mandating that all streams that support seeking also support >> >>concurrent >> >> >> >>random access seems like a high tax to impose on implementing >>Stream. >> >> >> >> >> >> >> >>If everyone were clamouring for it, then it'd be reasonable. But >> >> >> >>they're not. I've worked on a wide range of .NET projects in the >> >>years >> >> >> >>since .NET shipped, and while I can appreciate in abstract that >>this >> >> >> >>would be a nice feature, I've never run into a scenario where this >> >>would >> >> >> >>have been useful in practice. (But I have run into scenarios where >>I >> >> >> >>wanted to implement Stream. So this feature would have been a net >> >>loss >> >> >> >>for me.) >> >> >> >> >> >> >> >>More tellingly, you rarely see anyone asking for it on public >>forums. >> >>As >> >> >> >>far as I can tell from the various mailing lists, newsgroups, and >> >>forums >> >> >> >>I've hung out on, my experience of never having come across a >> >>concrete >> >> >> >>need for this is more common than your experience. >> >> >> >> >> >> >> >>Which suggests that Microsoft made the right decision: Stream >>would >> >>have >> >> >> >>been the wrong place to put support for concurrent random access. >> >> >> >> >> >> >> >>So even if you were starting from a clean slate, and wanted v1.0 >>of >> >>the >> >> >> >>framework to offer this functionality (and I agree it would be >>nice >> >>to >> >> >> >>have, despite the fact that I've never actually needed it) I still >> >>think >> >> >> >>it would be a mistake to put something this specialized into >> >>something >> >> >> >>as generic as Stream. >> >> >> >> >> >> >> >>It's at best a poor match (and at worst, inappropriate) for most >> >> >> >>implementations of Stream. For the implementations of Stream for >> >>which >> >> >> >>it's appropriate, it's not useful most of the time. >> >> >> >> >> >> >> >>To steal a phrase that Ron Jeffries recently used on the TDD >>mailing >> >> >> >>list: >> >> >> >> >> >> >> >> " it would be a bit like having a #10 Torx driver as part >> >> >> >> of my hammer. Occasionally useful but also an odd addition." >> >> >> >> >> >> >> >> >> >> >> >>Of course it's hugely frustrating if you happen to hit the case >>where >> >> >> >>you want to do something that you know would be pretty easy for >>the >> >> >> >>underlying implementation you happen to be using. But that doesn't >> >> >> >>translate into an argument that Stream is designed wrong. >> >> >> >> >> >> >> >> >> >>=================================== >> >> >>This list is hosted by DevelopMentor® http://www.develop.com >> >> >> >> >> >>View archives and manage your subscription(s) at >> >>http://discuss.develop.com >> >> > >> >> >=================================== >> >> >This list is hosted by DevelopMentor® http://www.develop.com >> >> > >> >> >View archives and manage your subscription(s) at >> >>http://discuss.develop.com >> >> >> >>=================================== >> >>This list is hosted by DevelopMentor® http://www.develop.com >> >> >> >>View archives and manage your subscription(s) at >>http://discuss.develop.com >> > >> >=================================== >> >This list is hosted by DevelopMentor® http://www.develop.com >> > >> >View archives and manage your subscription(s) at >>http://discuss.develop.com >> >>=================================== >>This list is hosted by DevelopMentor® http://www.develop.com >> >>View archives and manage your subscription(s) at http://discuss.develop.com > >=================================== >This list is hosted by DevelopMentor® http://www.develop.com > >View archives and manage your subscription(s) at http://discuss.develop.com =================================== This list is hosted by DevelopMentor® http://www.develop.com View archives and manage your subscription(s) at http://discuss.develop.com
