net: Implement canceling of all async operations in FileStream.

net/base/file_stream.h

Index: net/base/file_stream.h
===================================================================
--- net/base/file_stream.h	(revision 145483)
+++ net/base/file_stream.h	(working copy)
@@ -41,7 +41,16 @@
   // |net_log| is the net log pointer to use to create a |BoundNetLog|.  May be
   // NULL if logging is not needed.
   // The already opened file will not be automatically closed when FileStream
-  // is destructed.
+  // is destroyed.
+  // Note: if you use this constructor you have 3 options of further usage:
+  // - Only sync operations are used on this stream;
+  // - You wait for completion of all async operations;

[willchan no longer on Chromium, 2012/07/23 20:57:37]

Why? Why doesn't deleting FileStream automatically cancel all async operations?

[pivanof, 2012/07/23 21:18:29]

Well, sure it will "cancel". But cancellation means that this async operation
can be in progress at the moment of destructor execution. And besides being in
progress it can be currently at the point between check "if operation was
canceled" and the actual execution of operation. And if at this point user won't
transfer file descriptor ownership to FileStream (by means of calling of one of
the Close*() methods) and will close file descriptor by himself after
destruction of FileStream async operation can be executed on a stale file
descriptor.
So basically what I meant by this comment is user should either transfer
ownership by calling one of the Close*() methods or (if he doesn't want to do
that) wait for all async operations to complete. Maybe as there's no such use
case in current Chromium code and it's really tricky to understand it, maybe
this should be removed from the comment? I.e. we will say that if user creates
FileStream from file descriptor and calls async methods he must call Close*()
before destruction.

[willchan no longer on Chromium, 2012/07/23 21:32:02]

I think ownership is "transferred" when it is passed into the constructor. After
that point, it's a bug for the user to ever operate directly on the file
descriptor.

+  // - When you finished using stream you call any type of Close*() method
+  //   (including possibly CloseAndCancelAsync() or Close() with later
+  //   destruction without waiting for closing callback).
+  // If your usage doesn't fall into one of 3 options above you are introducing
+  // use-after-free bug when file descriptor could be used after you close it
+  // (and it possibly gets reopened to another file).
   FileStream(base::PlatformFile file, int flags, net::NetLog* net_log);
 
   // If the file stream was opened with Open() or OpenSync(), the underlying
@@ -62,10 +71,16 @@
 
   // Call this method to close the FileStream synchronously.
   // It is OK to call CloseSync() multiple times.  Redundant calls are
-  // ignored. Note that if there are any pending async operations, they'll
-  // be aborted.
+  // ignored. Note that if there are any pending async operations, then
+  // behavior of this method is equivalent to CloseAndCancelAsync().
   virtual void CloseSync();
 
+  // Call this method to cancel pending async operations and close FileStream
+  // asynchronously without any notification of completion. Method should be
+  // used when you want to quickly destroy FileStream without actually calling
+  // the destructor.
+  virtual void CloseAndCancelAsync();

[willchan no longer on Chromium, 2012/07/23 20:57:37]

I don't understand this...when do you want to quickly destroy FileStream without
actually calling the destructor?

[pivanof, 2012/07/23 21:18:29]

Looking at it now I think maybe I should delete this last sentence.
What I meant is behavior of this method is basically equivalent to calling
destructor (in terms of cancellation of async operations, asynchronous closing
of file descriptor without notification to user of the completion, and all that
without actual waiting for those operations to complete) but FileStream object
is not actually destroyed and still usable after that (it could be reopened if
necessary).

[willchan no longer on Chromium, 2012/07/23 21:32:02]

I'd rather get rid of this and have the user create a new FileStream if needed.

+
   // Call this method to open the FileStream asynchronously.  The remaining
   // methods cannot be used unless the file is opened successfully. Returns
   // ERR_IO_PENDING if the operation is started. If the operation cannot be