1

I have a Task that reads strings from a blocking collection and is supposed to write them out to a file. Trouble is, while the file is created, the size of the file is 0 bytes after the task completes.

While debugging, I see that non-empty lines are retrieved from the blocking collection, and the stream writer is wrapped in a using block.

For debugging I threw in a flush that should not be required and write the lines to the console. There are 100 non-empty lines of text read from the blocking collection.

// Stuff is placed in writeQueue from a different task BlockingCollection<string> writeQueue = new BlockingCollection<string>(); Task writer = Task.Factory.StartNew(() => { try { while (true) { using (FileStream fsOut = new FileStream(destinationPath, FileMode.Create, FileAccess.Write)) using (BufferedStream bsOut = new BufferedStream(fsOut)) using (StreamWriter sw = new StreamWriter(bsOut)) { string line = writeQueue.Take(); Console.WriteLine(line); // Stuff is written to the console sw.WriteLine(line); sw.Flush(); // Just in case, makes no difference } } } catch (InvalidOperationException) { // We're done. } });

Stepping through in the debugger, I see that the program terminates in an orderly manner. There are no unhandled exceptions.

What might be going wrong here?

Improve this question
3
  • Don't take an InvalidOperationException as a sign you are done reading or writing. This will hide real bugs. And also don't Flush because it does make a (negative) difference. Commented Oct 13, 2012 at 13:30
  • @usr: You actually have to handle InvalidOperationException. Even if one checks IsCompleted, the task can be preempted and the blocking collection marked complete just after that check. Or am I missing something? Also, that is the pattern on MSDN (not that it makes it necessarily good. After all MSDN hardly ever disposes of IDisposable resources in their examples, etc.). Commented Oct 16, 2012 at 1:34
  • 1
    You could use BlockingCollection.GetConsumingEnumerable. In any cas,e I'd wrap the try-catch around just that line and make an effort to arrange everything so that only this Take call is protected. Commented Oct 16, 2012 at 10:41

1 Answer 1

Reset to default
2

You are re-creating the file on every run of the loop. Change the FileMode.Create to FileMode.Append and it will keep the previous values you wrote on it.

Also, using exceptions to detect that you should stop is a really bad practice, if this a consumer-producer solution, you can easily do better by having the producer setting a thread safe flag variable signaling it has finished the work and will not produce anything else.

Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

Moving the while inside the usings would be much more efficient than .Append.
yup, not sure why he did it like that.
Geez. That's what I get for an 18 hour day :-) Thank you.
In fact, BlockingCollection<> already has a boolean property to indicate that it will not produce anything else: IsCompleted
@phoog: True, but you can still get the InvalidOperationException if you check that property, it is false, that thread is preempted by a thread that calls AddingComplete, and the original thread resumes. You still have to handle that exception. If I had many small blocking collections I might check the flag to improve efficiency and still handle the exception for the edge case. Since that's not the case for me, I went with the pattern in the MSDN examples.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.