paulyoder / LinqToExcel

Use LINQ to retrieve data from spreadsheets and csv files
MIT License
1.06k stars 299 forks source link

incorrect Dispose function #103

Closed PuskasJ closed 6 years ago

PuskasJ commented 8 years ago

Hi,

the code that actually makes the disposing of an ExcelQueryFactory is never called because of this condition on the begining of the function

    protected virtual void Dispose(bool disposing)
    {
        **if (!_disposed) // this is not ok, should be if (_disposed)
            return;**

           //this is never called because _disposed is always false
            if (disposing)
            {
                if (_queryArgs != null && _queryArgs.PersistentConnection != null)
                {
                    try
                    {
                        _queryArgs.PersistentConnection.Dispose();
                        _queryArgs.PersistentConnection = null;
                    }
                    catch (Exception ex)
                    {
                        _log.Error("Error disposing OleDbConnection", ex);
                    }
                }
            }

        _queryArgs = null;
        _disposed = true;
    }
buda56 commented 8 years ago

It also leaves a locked file until the application using the add-in is closed. In my circumstance I was using it in a service on a thread, once I accessed the file to read it thew file remained locked event though the thread and class had been terminated. As the service is still running the lock is maintained, doesn't release until the service is restarted.

Peter

sameerkattel commented 6 years ago

Only valid when UsePersistentConnection option is turned on.

using (var excel = new ExcelQueryFactory(UnclaimedPropertySourceInfo.FullName)) { excel.UsePersistentConnection = true; var worksheets = excel.GetWorksheetNames().OrderBy(i => i).ToList(); }

90OmarISC commented 6 years ago

Saben si en otra versión no se encuentra este problema

0xfeeddeadbeef commented 6 years ago

Unconfirmed? Here it is — the confirmation: https://github.com/paulyoder/LinqToExcel/blob/ff42b206fdc2ffbe7673eb2b320f5455c7b73720/src/LinqToExcel/ExcelQueryFactory.cs#L647-L648

drjerrard commented 6 years ago

I just came across this issue, myself, with the same conclusion. "if (!_disposed)" should be "if (_disposed)".

mrworkman commented 6 years ago

Fixed on master.

drjerrard commented 6 years ago

Is there a plan to move this to NuGet? 1.11.0 is still listed as current. That's from February 19th, 2017.

mrworkman commented 6 years ago

Yes, I plan on publishing a new one soon.

justinLangfeldt commented 4 years ago

Was this ever fixed, I see the code was merged to master however the file continues open and locked until the application is closed, dispose it not working correctly?

mrworkman commented 4 years ago

Ah, sorry. I've had a lot going on in the last year. It's been hard to get any time to work on this project :cry: