optimize file does not get closed #1

Closed
LaurinStrelow opened this Issue Feb 18, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@LaurinStrelow

Hi,

i think i found a bug. When using the the constant memory mode, the worksheet creates temporary files, as i can see this that files never get closed. After creating a large amount of worksheets (on iOS about 300), the system does not create new tempfiles and returns null instead, because there are too many (temp) files open.

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Feb 18, 2015

Owner

Hi,

The temp files aren't closed while the calling program is running since they may still be required for writing.

The Python version of the module has an undocumented facility to allow the user to close and if necessary re-open the temp files. This was to workaround a similar open file limit on Windows.

I'll look at adding something similar to libxlsxwriter.

John

Owner

jmcnamara commented Feb 18, 2015

Hi,

The temp files aren't closed while the calling program is running since they may still be required for writing.

The Python version of the module has an undocumented facility to allow the user to close and if necessary re-open the temp files. This was to workaround a similar open file limit on Windows.

I'll look at adding something similar to libxlsxwriter.

John

@jmcnamara jmcnamara self-assigned this Feb 18, 2015

@jmcnamara jmcnamara changed the title from optimize file does not get close to optimize file does not get closed Feb 18, 2015

@LaurinStrelow

This comment has been minimized.

Show comment
Hide comment
@LaurinStrelow

LaurinStrelow Feb 18, 2015

This sounds as a good solution. However at the moment the optimize files never get closed, even if the workbook gets closed or the worksheet freed and the file is not needed anymore.

This sounds as a good solution. However at the moment the optimize files never get closed, even if the workbook gets closed or the worksheet freed and the file is not needed anymore.

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Feb 19, 2015

Owner

However at the moment the optimize files never get closed, even if the workbook gets closed or the worksheet freed and the file is not needed anymore.

Yes. There is that issue as well. I'll fix it in the next few days.

Is that the issue that you are having or are you also trying to generate a workbook with > 300 worksheets?

John

Owner

jmcnamara commented Feb 19, 2015

However at the moment the optimize files never get closed, even if the workbook gets closed or the worksheet freed and the file is not needed anymore.

Yes. There is that issue as well. I'll fix it in the next few days.

Is that the issue that you are having or are you also trying to generate a workbook with > 300 worksheets?

John

@LaurinStrelow

This comment has been minimized.

Show comment
Hide comment
@LaurinStrelow

LaurinStrelow Feb 19, 2015

Yes this was the issue i have. I make two workbooks with about 200 worksheets and the second workbook won't complete. But i think it is better to have the possibility to close the file and won't get near the file limit anytime later, if more worksheets are needed.

Yes this was the issue i have. I make two workbooks with about 200 worksheets and the second workbook won't complete. But i think it is better to have the possibility to close the file and won't get near the file limit anytime later, if more worksheets are needed.

@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Feb 23, 2015

Owner

Test case:

#include "xlsxwriter.h"

int main() {
    int i;
    lxw_workbook  *workbook;
    lxw_worksheet *worksheet;
    lxw_workbook_options options = {.constant_memory = 1};

    for (i = 0; i <= 5000; i++) {
        printf("File = %d\n", i);

        workbook  = new_workbook_opt("test.xlsx", &options);
        worksheet = workbook_add_worksheet(workbook, NULL);

        worksheet_write_string(worksheet, 0, 0, "Hello", NULL);
        worksheet_write_number(worksheet, 1, 0, 123, NULL);

        workbook_close(workbook);
    }

    return 0;
}
Owner

jmcnamara commented Feb 23, 2015

Test case:

#include "xlsxwriter.h"

int main() {
    int i;
    lxw_workbook  *workbook;
    lxw_worksheet *worksheet;
    lxw_workbook_options options = {.constant_memory = 1};

    for (i = 0; i <= 5000; i++) {
        printf("File = %d\n", i);

        workbook  = new_workbook_opt("test.xlsx", &options);
        worksheet = workbook_add_worksheet(workbook, NULL);

        worksheet_write_string(worksheet, 0, 0, "Hello", NULL);
        worksheet_write_number(worksheet, 1, 0, 123, NULL);

        workbook_close(workbook);
    }

    return 0;
}

jmcnamara added a commit that referenced this issue Feb 23, 2015

Fix for open tmpfiles in constant_memory mode.
Use implicit close for all tmpfiles in constant_memory mode.

Issue #1.
@jmcnamara

This comment has been minimized.

Show comment
Hide comment
@jmcnamara

jmcnamara Feb 23, 2015

Owner

Hi,

I've pushed a fix to master for the tmpfile issue in constant_memory mode. You can try it when you get a chance.

You should open a second, feature request, issue for the extension to open/close the tempfiles during the life of the workbook, if you need that feature.

P.S. For iOS did you have to use downgrade the version of the zlib libraries as shown here?

John

Owner

jmcnamara commented Feb 23, 2015

Hi,

I've pushed a fix to master for the tmpfile issue in constant_memory mode. You can try it when you get a chance.

You should open a second, feature request, issue for the extension to open/close the tempfiles during the life of the workbook, if you need that feature.

P.S. For iOS did you have to use downgrade the version of the zlib libraries as shown here?

John

@LaurinStrelow

This comment has been minimized.

Show comment
Hide comment
@LaurinStrelow

LaurinStrelow Feb 24, 2015

Hi,

thank you! This solves my issue. I will open another issue for the feature request.
I did not had to downgrade the zlib. The app has already mailcore installed via cocoapods, their binary includes minizip. Therefore i did not need to install minizip or take care of zlib. They are using the preinstalled library. However i needed to replace the zipOpenNewFileInZip4_64 with zipOpenNewFileInZip2 to get the linking done. This is sadly the only possibility i have.

Hi,

thank you! This solves my issue. I will open another issue for the feature request.
I did not had to downgrade the zlib. The app has already mailcore installed via cocoapods, their binary includes minizip. Therefore i did not need to install minizip or take care of zlib. They are using the preinstalled library. However i needed to replace the zipOpenNewFileInZip4_64 with zipOpenNewFileInZip2 to get the linking done. This is sadly the only possibility i have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment