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
