ext/pdo_firebird: Fixed GH-18276 - persistent connection - "zend_mm_heap corrupted" with setAttribute()
#18280
Conversation
SakiTakamachi
force-pushed
the
fix/gh18276_3
branch
3 times, most recently
from
e175d97 to
bd8ad0e
Compare
SakiTakamachi
force-pushed
the
fix/gh18276_3
branch
from
bd8ad0e to
1c25919
Compare
|
Ah, the cause appears to be a mixture of persistent and non-persistent memory. It's too late now so I'll fix it tomorrow. |
ext/pdo_firebird/firebird_driver.c
Outdated
| @@ -492,13 +492,16 @@ static void firebird_handle_closer(pdo_dbh_t *dbh) /* {{{ */ | |||
| } | |||
|
|
|||
| if (H->date_format) { | |||
| efree(H->date_format); | |||
| pefree(H->date_format, dbh->is_persistent); | |||
| H->date_format = NULL; | |||
There was a problem hiding this comment.
Since you're freeing the entire structure anyway below, you don't need to set the pointers for these 3 fields to NULL.
| @@ -881,9 +884,10 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval * | |||
| return false; | |||
| } | |||
| if (H->date_format) { | |||
| efree(H->date_format); | |||
| pefree(H->date_format, dbh->is_persistent); | |||
| H->date_format = NULL; | |||
There was a problem hiding this comment.
I'm okay with these being set to NULL because technically we could bail out if the pestrdup below fails.
ext/pdo_firebird/firebird_driver.c
Outdated
| } | ||
| spprintf(&H->date_format, 0, "%s", ZSTR_VAL(str)); | ||
| H->date_format = pestrdup(ZSTR_VAL(str), dbh->is_persistent); |
There was a problem hiding this comment.
Since we know the length, you should use pestrndup instead and pass in ZSTR_LEN(str) as well (same below).
| PDO::ATTR_PERSISTENT => true, | ||
| ], | ||
| ); | ||
| // Avoid interned |
There was a problem hiding this comment.
str_repeat is special cased in the SCCP optimization pass if you have a constant number as a second argument, and so you'll still get an interned string here.
You need to do this: str_repeat('my string', random_int(1, 1));
|
@nielsdos |
fixes #18276
The structure has changed slightly on master, so I don’t plan to apply the change the third commit to master.
I’ll follow up with a separate PR for changes targeting master.