[GH-ISSUE #453] It seems there is a memory leak in constant memory enabled when trying to insert date #356

Closed
opened 2026-05-05 12:11:41 -06:00 by gitea-mirror · 7 comments
Owner

Originally created by @videni on GitHub (Aug 27, 2024).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/453

Sorry, I don't know much about C. I can't reproduce it with C. I only found the problem in here. I copy the critical code here if you can spot it quickly. The code enabled const memory is not copied here, but it is enabled in my local. The code looks very normal.

PHP_METHOD(vtiful_xls, insertDate)
{
    zval *data = NULL, *format_handle = NULL;
    zend_long row = 0, column = 0;
    zend_string *format = NULL;

    ZEND_PARSE_PARAMETERS_START(3, 5)
            Z_PARAM_LONG(row)
            Z_PARAM_LONG(column)
            Z_PARAM_ZVAL(data)
            Z_PARAM_OPTIONAL
            Z_PARAM_STR_OR_NULL(format)
            Z_PARAM_RESOURCE_OR_NULL(format_handle)
    ZEND_PARSE_PARAMETERS_END();

    ZVAL_COPY(return_value, getThis());

    xls_object *obj = Z_XLS_P(getThis());

    WORKBOOK_NOT_INITIALIZED(obj);
    SHEET_LINE_SET(obj, row);

    if (Z_TYPE_P(data) != IS_LONG) {
        zend_throw_exception(vtiful_exception_ce, "timestamp is long", 160);
        return;
    }

    // Default datetime format
    if (format == NULL || (format != NULL && ZSTR_LEN(format) == 0)) {
        format = zend_string_init(ZEND_STRL("yyyy-mm-dd hh:mm:ss"), 0);
    }

    lxw_datetime datetime = timestamp_to_datetime(data->value.lval);

    if (format_handle != NULL) {
        datetime_writer(&datetime, row, column, format, &obj->write_ptr, zval_get_format(format_handle));
    } else {
        datetime_writer(&datetime, row, column, format, &obj->write_ptr, obj->format_ptr.format);
    }

    // Release default format
    if (ZEND_NUM_ARGS() == 3) {
        zend_string_release(format);
    }
}
/*
 * Write the datetime to the file
 */
void datetime_writer(lxw_datetime *datetime, zend_long row, zend_long columns, zend_string *format, xls_resource_write_t *res, lxw_format *format_handle)
{
    lxw_format *value_format = workbook_add_format(res->workbook);

    if (format_handle != NULL) {
        format_copy(value_format, format_handle);
    }

    format_set_num_format(value_format, ZSTR_VAL(format));
    worksheet_write_datetime(res->worksheet, (lxw_row_t)row, (lxw_col_t)columns, datetime, value_format);
}
Originally created by @videni on GitHub (Aug 27, 2024). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/453 Sorry, I don't know much about C. I can't reproduce it with C. I only found the problem in [here](https://github.com/viest/php-ext-xlswriter/issues/455). I copy the critical code here if you can spot it quickly. The code enabled const memory is not copied here, but it is enabled in my local. The code looks very normal. ``` PHP_METHOD(vtiful_xls, insertDate) { zval *data = NULL, *format_handle = NULL; zend_long row = 0, column = 0; zend_string *format = NULL; ZEND_PARSE_PARAMETERS_START(3, 5) Z_PARAM_LONG(row) Z_PARAM_LONG(column) Z_PARAM_ZVAL(data) Z_PARAM_OPTIONAL Z_PARAM_STR_OR_NULL(format) Z_PARAM_RESOURCE_OR_NULL(format_handle) ZEND_PARSE_PARAMETERS_END(); ZVAL_COPY(return_value, getThis()); xls_object *obj = Z_XLS_P(getThis()); WORKBOOK_NOT_INITIALIZED(obj); SHEET_LINE_SET(obj, row); if (Z_TYPE_P(data) != IS_LONG) { zend_throw_exception(vtiful_exception_ce, "timestamp is long", 160); return; } // Default datetime format if (format == NULL || (format != NULL && ZSTR_LEN(format) == 0)) { format = zend_string_init(ZEND_STRL("yyyy-mm-dd hh:mm:ss"), 0); } lxw_datetime datetime = timestamp_to_datetime(data->value.lval); if (format_handle != NULL) { datetime_writer(&datetime, row, column, format, &obj->write_ptr, zval_get_format(format_handle)); } else { datetime_writer(&datetime, row, column, format, &obj->write_ptr, obj->format_ptr.format); } // Release default format if (ZEND_NUM_ARGS() == 3) { zend_string_release(format); } } ``` ``` /* * Write the datetime to the file */ void datetime_writer(lxw_datetime *datetime, zend_long row, zend_long columns, zend_string *format, xls_resource_write_t *res, lxw_format *format_handle) { lxw_format *value_format = workbook_add_format(res->workbook); if (format_handle != NULL) { format_copy(value_format, format_handle); } format_set_num_format(value_format, ZSTR_VAL(format)); worksheet_write_datetime(res->worksheet, (lxw_row_t)row, (lxw_col_t)columns, datetime, value_format); } ```
Author
Owner

@jmcnamara commented on GitHub (Aug 27, 2024):

  • What OS are you using?
  • What hardware is it?
  • On the OS is the /tmp directory mapped to system memory?
  • Are you seeing a "leak" where the application doesn't return memory to the OS after it completes?
  • Or are you seeing a situation where the application just consumes memory as it runs?

If it is the last one and your /tmp is mapped to memory then the application is consuming disk space but that disk space has been mapped to memory.

If this is the case then libxlsxwriter has an option to change the location of the temp files it used:

http://libxlsxwriter.github.io/workbook_8h.html#a7329be90faba9e9ee65f58cb221aa4f1

Check if php-ext-xlsxwriter has the same opinion.

Note, libxlsxwriter has hundreds of tests for memory leaks that run in the CI so it is unlikely that it is an actual leak.

<!-- gh-comment-id:2311579962 --> @jmcnamara commented on GitHub (Aug 27, 2024): - What OS are you using? - What hardware is it? - On the OS is the /tmp directory mapped to system memory? - Are you seeing a "leak" where the application doesn't return memory to the OS after it completes? - Or are you seeing a situation where the application just consumes memory as it runs? If it is the last one **and** your /tmp is mapped to memory then the application is consuming disk space but that disk space has been mapped to memory. If this is the case then libxlsxwriter has an option to change the location of the temp files it used: http://libxlsxwriter.github.io/workbook_8h.html#a7329be90faba9e9ee65f58cb221aa4f1 Check if php-ext-xlsxwriter has the same opinion. Note, libxlsxwriter has hundreds of tests for memory leaks that run in the CI so it is unlikely that it is an actual leak.
Author
Owner

@videni commented on GitHub (Aug 27, 2024):

@jmcnamara, thansk for the tips, there is no option to change the location of the temp files in the php-ext-xlsxwriter , and the problem it not the tmp files, I run the code on MacOS, Ubuntu , Alpine linux, all have the issue.

I think it can reproduced by this link, you can observe continious growth of memory when constant memory enabled and insert 1 million rows with a datetime. sorry it is PHP code.

<!-- gh-comment-id:2311638213 --> @videni commented on GitHub (Aug 27, 2024): @jmcnamara, thansk for the tips, there is no option to change the location of the temp files in the php-ext-xlsxwriter , and the problem it not the tmp files, I run the code on MacOS, Ubuntu , Alpine linux, all have the issue. I think it can reproduced by this [link](https://github.com/viest/php-ext-xlswriter/issues/455#issuecomment-2311511661), you can observe continious growth of memory when constant memory enabled and insert 1 million rows with a datetime. sorry it is PHP code.
Author
Owner

@jmcnamara commented on GitHub (Aug 27, 2024):

Ok. Sorry I can't help you other than the advice above.

<!-- gh-comment-id:2311650159 --> @jmcnamara commented on GitHub (Aug 27, 2024): Ok. Sorry I can't help you other than the advice above.
Author
Owner

@videni commented on GitHub (Aug 27, 2024):

@jmcnamara after some experiments, I found this line caused the memory leak. does the code below remind you something please?

the below are from the datetime_writer method above.

// it might this line. because  I inserted 1 millon datetime object,  the value_format will be created that much, 
// I think somehow, this object not released in const memory. 
 lxw_format *value_format = workbook_add_format(res->workbook);

  if (format_handle != NULL) {
      format_copy(value_format, format_handle);
  }
<!-- gh-comment-id:2311810872 --> @videni commented on GitHub (Aug 27, 2024): @jmcnamara after some experiments, I found this line caused the memory leak. does the code below remind you something please? the below are from the `datetime_writer` method above. ``` // it might this line. because I inserted 1 millon datetime object, the value_format will be created that much, // I think somehow, this object not released in const memory. lxw_format *value_format = workbook_add_format(res->workbook); if (format_handle != NULL) { format_copy(value_format, format_handle); } ```
Author
Owner

@jmcnamara commented on GitHub (Aug 27, 2024):

That definitely looks like an issue. It seems to be creating a new format object for each call to datetime_writer(). These objects are cleaned up on exit so it isn't a leak. However, it will cause a memory growth. Especially, if you writing a million cells.

You should contact the author and let them know.

You may be able to work around it by writing a number instead of a date. In Excel a date is a number with a format. You will need to covert the dates yourself but it will avoid this issue (unless the number writer is doing the same thing).

See: https://libxlsxwriter.github.io/working_with_dates.html

<!-- gh-comment-id:2312091863 --> @jmcnamara commented on GitHub (Aug 27, 2024): That definitely looks like an issue. It seems to be creating a new format object for each call to `datetime_writer()`. These objects are cleaned up on exit so it isn't a **leak**. However, it will cause a memory **growth**. Especially, if you writing a million cells. You should contact the author and let them know. You may be able to work around it by writing a number instead of a date. In Excel a date is a number with a format. You will need to covert the dates yourself but it will avoid this issue (unless the number writer is doing the same thing). See: https://libxlsxwriter.github.io/working_with_dates.html
Author
Owner

@videni commented on GitHub (Aug 27, 2024):

@jmcnamara I see, thanks a lot. Finally, I understand what the real problem is. The php-ext-xlswriter almost calls the workbook_add_format method for each cell. that issue existed for 2 years, still not fixed. Some design issues might exist , not easy to fix. There might be two ways to solve the problem:

  1. Reuse existing from upper level code.
  2. Or don't create format for each cell, but use the format set by set_column method instead.

except strings, the number has the same issue as datetime, if no format passed for the worksheet_write_string, worksheet_write_number, worksheet_write_datetime, it will use the format set by theset_column method , right?

<!-- gh-comment-id:2312364365 --> @videni commented on GitHub (Aug 27, 2024): @jmcnamara I see, thanks a lot. Finally, I understand what the real problem is. The `php-ext-xlswriter` almost calls the `workbook_add_format ` method for each cell. that issue existed for 2 years, still not fixed. Some design issues might exist , not easy to fix. There might be two ways to solve the problem: 1. Reuse existing from upper level code. 2. Or don't create format for each cell, but use the format set by `set_column` method instead. except strings, the `number` has the same issue as `datetime`, if no format passed for the `worksheet_write_string`, `worksheet_write_number`, `worksheet_write_datetime`, it will use the format set by the`set_column` method , right?
Author
Owner

@videni commented on GitHub (Aug 28, 2024):

Closed as solved by php-ext-xlswriter, @jmcnamara thanks for help,

<!-- gh-comment-id:2314174399 --> @videni commented on GitHub (Aug 28, 2024): Closed as solved by php-ext-xlswriter, @jmcnamara thanks for help,
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/libxlsxwriter#356
No description provided.