Saving a Workbook and Re-opening It Corrupts the File about npoi HOT 13 CLOSED

I downgraded to 2.6.2 and the problem is still there (i.e., create an xlsx file via NPOI, open it, save it without making any changes, open it via NPOI and just save it again without making any changes).

from npoi.

Message=The process cannot access the file 'C:\Programming\SSMC LGL\exports\Reliability.xlsx' because it is being used by another process.

This means the file is occupied by Excel or something else. Please check if you keep it open in Excel or not.

And Excel is not a plain text file. You cannot just reopen the existing filestream and append the changed data with IWorkbook.Write. This is not the way. You have to replace the file or recreate the file to update the existing Excel file.

from npoi.

Did you try 2.6.2? I need to confirm if it's a regression bug.

from npoi.

No, but I will when I get the time, probably tomorrow.

Thanx for the quick reply.

from npoi.

I tried to Load and Save the file twice with LoadAndSaveBack tool with the latest NPOI code.

It looks the output files are correct and Excel can open these files.

What does the following code do in your code? Is it possible that it changed something?

foreach( var kvp in _sheets )
{
       kvp.Value.Export( workbook, createFile );
 }
var sheetNames = new List<string>();

foreach( var sheet in _sheetOrder ?? Enumerable.Empty<string>() )
{
    if( _sheets.TryGetValue( sheet, out var toExport ) )
        sheetNames.Add( toExport.SheetName );
}

if( sheetNames.Count == _sheets.Count )
{
    for( var idx = 0; idx < sheetNames.Count; idx++ )
    {
        workbook.SetSheetOrder( sheetNames[ idx ], idx );
    }
}

from npoi.

:) It does quite a bit. Most of the activity occurs in the first foreach, with each iteration generating (or updating) a worksheet. createFile controls whether a sheet is updated or created from scratch. I do this because sometimes the workbooks to which I'm exporting data have sheets which use the data I'm exporting (there's a separate bug report I filed about how the workbook cloning process does not currently copy chartsheets).

I don't run into problems when I create sheets, only when I'm updating an existing sheet. Here's what happens during an update.

    public void Export( IWorkbook workbook, bool workbookIsNew )
    {
        Sheet = workbookIsNew ? workbook.CreateSheet( SheetName ) : workbook.ClearSheet( SheetName );

        // if any columns have a top vertical alignment they all should have it
        if( _columns.Any( c => c.StyleSet.VerticalAlignment == VerticalAlignment.Top ) )
        {
            foreach( var column in _columns )
            {
                column.StyleSet.VerticalAlignment = VerticalAlignment.Top;
            }
        }

        BuildWorksheet();

        if (workbookIsNew)
            CreateNamedRanges();
        else UpdateNamedRanges(workbook);
    }

The key difference is that existing worksheets get cleared, via ClearSheet(), rather than created.

    public static ISheet ClearSheet( this IWorkbook workbook, string sheetName )
    {
        var retVal = workbook.GetSheet( sheetName );

        if( retVal == null )
        {
            Log.Warning( "Could not find sheet '{sheet}' to update, creating new ISheet", sheetName );
            return workbook.CreateSheet( sheetName );
        }

        // clear the sheet...but not the named ranges, which we will adjust
        // when we're done adding data to the sheet
        for( var rowIdx = 0; rowIdx <= retVal.LastRowNum; rowIdx++ )
        {
            var row = retVal.GetRow( rowIdx );
            if( row == null || row.FirstCellNum < 0)
                continue;

            for( var colIdx = row.FirstCellNum; colIdx < row.LastCellNum; colIdx++ )
            {
                var cell = row.GetCell( colIdx );
                cell?.SetBlank();
            }
        }

        return retVal;
    }

ClearSheet() is implemented as a static extension method so I can use it in other scenarios. All it does is call SetBlank() on every cell that has content in the worksheet being cleared.

Hope this helps. Let me know if you need/want more information. Exporting content to the sheet is more involved than what I've shown here, but it's the same code in both the "create a new sheet" and "update an existing sheet" scenarios.

from npoi.

Can you please also try to save the doc to MemoryStream instead of FileStream

using(var writeStream = new MemoryStream())
{
  workbook.Write(writeStream);
  workbook.Close();
  File.WriteAllBytes(filePath, writeStream.ToArray());
}

from npoi.

That appears to have solved the problem.

I'm curious, though, as to why writing the file from a MemoryStream worked. It seems like an unnecessary extra step.

BTW, I've noticed another problem when recreating/updating sheets -- cells with formulas don't display the formula results, although the formula is present -- but I'm not sure that's related to the issue raised in this bug report. I'll open another report if I determine it's not due to something I'm doing.

from npoi.

Can you please rollback this change and instead initialize the workbook with XSSFWorkbook constructor, not from FileStream:

workbook = new XSSFWorkbook(filePath);

from npoi.

When I initialize from the XSSFWorkbook constructor I get an error when I try to write the workbook to the same file name (after modifying it) because the XSSFWorkbook constructor apparently doesn't close the stream it opens to read the file.

from npoi.

Well, I believe it's not an NPOI bug, try this code and check if it's working:

using(FileStream writeStream = new FileStream(filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None)) 
{  
   writeStream.SetLength(0);
   workbook.Write(writeStream);
   workbook.Close();
}  

from npoi.

Same exception:

System.IO.IOException
HResult=0x80070020
Message=The process cannot access the file 'C:\Programming\SSMC LGL\exports\Reliability.xlsx' because it is being used by another process.
Source=System.Private.CoreLib
StackTrace:
at Microsoft.Win32.SafeHandles.SafeFileHandle.CreateFile(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)
at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable1 unixCreateMode) at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable1 unixCreateMode)
at System.IO.Strategies.FileStreamHelpers.ChooseStrategyCore(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize, Nullable`1 unixCreateMode)
at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share)
at J4JSoftware.Lgl.Common.WorkbookCreator.Export(String filePath, Boolean forceRecreation) in C:\Programming\SSMC LGL\Common\exporters\WorkbookCreator.cs:line 103
at TestProject.ExportPhase.Reliability() in C:\Programming\SSMC LGL\TestProject\phases\ExportPhase.cs:line 318
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

The code to read the existing workbook is:

        if( File.Exists( filePath ) && !forceRecreation )
        {
            createFile = false;
            FileStream? readStream = null;

            try
            {
                workbook = new XSSFWorkbook(filePath);

                //readStream = new FileStream( filePath, FileMode.Open, FileAccess.Read );

                //workbook = new XSSFWorkbook( readStream );
                //readStream.Close();
                //readStream.Dispose();
            }
            catch( Exception ex )
            {
                Log.Warning( "Could not read file '{file}', message was '{mesg}'; deleting and re-creating instead",
                             filePath,
                             ex.Message );

                workbook?.Close();
                workbook = null;

                readStream?.Dispose();

                File.Delete( filePath );
                createFile = true;
            }
        }

        workbook ??= new XSSFWorkbook();

I realize that could be significantly shorter, but I don't want to lose the earlier, working approach.

The code to write the file is:

        using( var writeStream =
              new FileStream( filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None ) )
        {
            writeStream.SetLength( 0 );
            workbook.Write( writeStream );
            workbook.Close();
        }

The exception is thrown by the FileStream constructor.

from npoi.

@markolbert, I can't test the code right now, but the main idea is when you writing to the existing filestream NPOI appends the data from the beginning and if the new file is shorter, the bytes at the end are not removed. It cause file corruption for some programs. As already said, you shouldn't write to the existing filestream. Try to undo all the changes and add only this line in your code when writing to the stream

writeStream.SetLength(0);

from npoi.