Rolling back file errors

[valentin tihomirov]

Once file is open, it should be closed:

Stream fs = new FileStream(filename, FileMode.Create);
try {
// write, exceptions may raise here
} finally {
fs.Close();
}

What should one do to remove the file in case of an error?

[TS25]

What should one do to remove the file in case of an error?

Stream fs = new FileStream(filename, FileMode.Create);
try
{
//do sth
}
catch (IOException)
{
//sth went wrong
File.Delete(filename);
}
finally
{
fs.Close()
}

[Jon Skeet [C# MVP]]

On May 8, 2:13 pm, "TS25" <[e-mail]:

What should one do to remove the file in case of an error?

Stream fs = new FileStream(filename, FileMode.Create);
try
{
//do sth}

catch (IOException)
{
//sth went wrong
File.Delete(filename);}

finally
{
fs.Close()

}

That's unlikely to work, as you'll be closing the filestream *after*
trying to delete the file - I'd expect the call to File.Delete to
fail. Either close the stream in the catch block directly as well, or
keep a flag of whether or not to delete the file, set it in the catch,
and do it in the finally.

Jon

[Willy Denoyette [MVP]]

"TS25" <xx*@xxx.comwrote in message
news:eW**************@TK2MSFTNGP02.phx.gbl...

>What should one do to remove the file in case of an error?

Stream fs = new FileStream(filename, FileMode.Create);
try
{
//do sth
}
catch (IOException)
{
//sth went wrong
File.Delete(filename);
}
finally
{
fs.Close()
}

This will not work as you can't delete a file which is still open. So,
you'll have to close the file before calling Delete.
What you can do is set a flag in the catch handler, in the finally clause
you can check the flag ( after calling Close) and delete the file when set .

Willy.

[valentin tihomirov]

fail. Either close the stream in the catch block directly as well, or
keep a flag of whether or not to delete the file, set it in the catch,
and do it in the finally.

Peahaps I misunderstand your idea, but I contrive to do it vice-versa -- set
up the flag in finally and rollback in catch:

enum RollbackMarker {none, file, db_record};
RollbackMarker rbm = RollbackMarker.none; // rollback marker
try {
// create a file
Stream fs = new FileStream(name, FileMode.Create);
try {
// write file
} finally {
fs.Close(); // first, we must close the file
rmb = RollbackMarker.file; // 2nd, we must remove it if
failure
}

// add DB entry
Entry entry = db.insert(data);
rbm = RollbackMarker.db_record;

.. make more construction-allocations

} catch (Exception e) {
// failure, rollback the allocations
log("rolling back since " + rbm);
switch (rbm) {
...
case RollbackMarker.db_record: File.Delete(); goto case
RollbackMarker.file; // why the hell did MS refuse the downfalls?
case RollbackMarker.file: db.remove(entry); break;
}
throw; // re-throw
}
Note, the rollback marker in the finally section.Whether file I/O success or
error we must complete the file creation by closing it and and note that the
file has been created. Resources must be closed only if their
constructor/allocator has succeeded.Therefore, I set up the marker after
construction.

[Jon Skeet [C# MVP]]

valentin tihomirov <V_*********@best.eewrote:

fail. Either close the stream in the catch block directly as well, or
keep a flag of whether or not to delete the file, set it in the catch,
and do it in the finally.

Peahaps I misunderstand your idea, but I contrive to do it vice-versa -- set
up the flag in finally and rollback in catch

That seems significantly more complicated to me.

Oh, and MS removed fallthrough in switch cases because almost all
occurrences of it in languages which allow it are accidental rather
than deliberate. Admittedly switch should have been radically
overhauled anyway, but never mind...

If you want your enum to effectively have flags as to whether to remove
the file and whether to remove the database record, then *implement* it
as a flags enum, with 1=file, 2=db_record and the implicit 3=both. (I'd
also recommend using .NET naming conventions, but that's just an
aside.)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[Nicholas Paldino [.NET/C# MVP]]

Personally, I'd do something like this:

public struct FileRollback : IDisposable
{
// The file name.
private string filename;

// Store the file.
public FileRollback(string filename)
{
// Set the file.
this.filename = filename;

// Delete by default.
rollback = true;
}

// Rollback the file?
private bool rollback;

public void Commit()
{
// Do not rollback the file.
rollback = false;
}

public void Dispose()
{
// If rolling back the file, delete here.
if (rollback)
{
// Delete.
File.Delete(filename);
}
}
}

Then, in the client code:

// Create the rollback.
using (FileRollback fileRollback = new FileRollback(filename))
using (Stream fs = new FileStream(filename, FileMode.Create))
{
// Do your work.

// Do not delete the file at this point.
fileRollback.Commit();
}

This way, if an exception is thrown, the file is deleted, if not, the
file remains. It also cleans up the code a great deal.
--
- Nicholas Paldino [.NET/C# MVP]
- mv*@spam.guard.caspershouse.com

"Jon Skeet [C# MVP]" <sk***@pobox.comwrote in message
news:MP********************@msnews.microsoft.com.. .

valentin tihomirov <V_*********@best.eewrote:

fail. Either close the stream in the catch block directly as well, or
keep a flag of whether or not to delete the file, set it in the catch,
and do it in the finally.

Peahaps I misunderstand your idea, but I contrive to do it vice-versa --
set
up the flag in finally and rollback in catch

That seems significantly more complicated to me.

Oh, and MS removed fallthrough in switch cases because almost all
occurrences of it in languages which allow it are accidental rather
than deliberate. Admittedly switch should have been radically
overhauled anyway, but never mind...

If you want your enum to effectively have flags as to whether to remove
the file and whether to remove the database record, then *implement* it
as a flags enum, with 1=file, 2=db_record and the implicit 3=both. (I'd
also recommend using .NET naming conventions, but that's just an
aside.)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

[valentin tihomirov]

That seems significantly more complicated to me.

Of course, if you have only one object to rollback a bit flag suffices to
you. I have presented a whole framework to rollback all allocations till
error point. It conforms the construction-deconstruction framework I
proposed in this incomplete blogg:
http://valjok.blogspot.com/2007/05/r...cessfully.html

>
Oh, and MS removed fallthrough in switch cases because almost all
occurrences of it in languages which allow it are accidental rather
than deliberate. Admittedly switch should have been radically
overhauled anyway, but never mind...

"Accidential" means here a "naturally simple grammar". Now, you add an
artificial barrier adding a level of complexity into grammar in order to
preclude user freedom. For me it looks like you had a natural direct access
from A to B and after adding some restricting rules you should explicitly
ensure every movement. Direct access means you can write less code, less
effort to reach some goal. Not sure the overcomplication worth breaking this
natural goodness.

[valentin tihomirov]

I have did it this way:

Stream tempFile = createfile();
try {
try {
// fill with data
} finally { // always close first
tempFile.Close();
}
// use
} finally { // always remove the closed file
File.Remove(tempFile.Name); // always remove the closed file
}
Actually, I required a bit more complex task:

Stream tempFile = createfile();
try {
try {
//fill with data
} finally { // always close first
tempFile.Close();
}

// process the file deriving its persistent name
// File.Move(tempFile, persistentName);
} catch { // remove the file if it has failed to rename
File.Remove(tempFile.Name);
throw;
}
The only consern is if renaming succeeded but an error occured nevertheless.
Is it possible?