Skip to content

Commit 887b793

Browse files
kocsismatekocsismate
committed
Hopefully fix all memleaks
1 parent 4282362 commit 887b793

4 files changed

Lines changed: 128 additions & 33 deletions

File tree

‎ext/odbc/php_odbc.c‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ static int _close_pconn_with_res(zval *zv, void *p)
102102
}
103103

104104
odbc_connection *list_conn = ((odbc_connection *)(le->ptr));
105-
odbc_connection *obj_conn = odbc_link_from_obj((zend_object*)p)->connection;
105+
odbc_connection *obj_conn = (odbc_connection *) p;
106106
if (list_conn == obj_conn) {
107107
return ZEND_HASH_APPLY_REMOVE;
108108
}
@@ -138,10 +138,10 @@ static void odbc_link_free(odbc_link *link)
138138
zend_hash_destroy(&link->connection->results);
139139
efree(link->connection);
140140
ODBCG(num_links)--;
141+
}
141142

142-
if (link->hash) {
143-
zend_hash_del(&ODBCG(non_persistent_connections), link->hash);
144-
}
143+
if (link->hash) {
144+
zend_hash_del(&ODBCG(connections), link->hash);
145145
}
146146

147147
link->connection = NULL;
@@ -317,6 +317,7 @@ static void _close_odbc_pconn(zend_resource *rsrc)
317317
SQLFreeConnect(conn->hdbc);
318318
SQLFreeEnv(conn->henv);
319319
}
320+
free(conn);
320321

321322
ODBCG(num_links)--;
322323
ODBCG(num_persistent)--;
@@ -501,12 +502,12 @@ static PHP_GINIT_FUNCTION(odbc)
501502
ZEND_TSRMLS_CACHE_UPDATE();
502503
#endif
503504
odbc_globals->num_persistent = 0;
504-
zend_hash_init(&odbc_globals->non_persistent_connections, 0, NULL, NULL, 1);
505+
zend_hash_init(&odbc_globals->connections, 0, NULL, NULL, 1);
505506
}
506507

507508
static PHP_GSHUTDOWN_FUNCTION(odbc)
508509
{
509-
zend_hash_destroy(&odbc_globals->non_persistent_connections);
510+
zend_hash_destroy(&odbc_globals->connections);
510511
}
511512

512513
/* {{{ PHP_MINIT_FUNCTION */
@@ -878,14 +879,14 @@ PHP_FUNCTION(odbc_close_all)
878879
}
879880

880881
/* Loop through the non-persistent connection list, now close all non-persistent connections and their results */
881-
ZEND_HASH_FOREACH_VAL(&ODBCG(non_persistent_connections), zv) {
882+
ZEND_HASH_FOREACH_VAL(&ODBCG(connections), zv) {
882883
odbc_link *link = Z_ODBC_LINK_P(zv);
883884
if (link->connection) {
884885
odbc_link_free(link);
885886
}
886887
} ZEND_HASH_FOREACH_END();
887888

888-
zend_hash_clean(&ODBCG(non_persistent_connections));
889+
zend_hash_clean(&ODBCG(connections));
889890

890891
zend_hash_apply(&EG(persistent_list), _close_pconn);
891892
}
@@ -2299,7 +2300,7 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
22992300
}
23002301

23012302
char *hashed_details;
2302-
int hashed_details_len = spprintf(&hashed_details, 0, "%s_%s_%s_%s_%d", ODBC_TYPE, db, uid, pwd, cur_opt);
2303+
int hashed_details_len = spprintf(&hashed_details, 0, "%d_%s_%s_%s_%s_%d", persistent, ODBC_TYPE, db, uid, pwd, cur_opt);
23032304

23042305
try_and_get_another_connection:
23052306

@@ -2333,6 +2334,8 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
23332334
RETURN_FALSE;
23342335
}
23352336

2337+
zend_hash_str_add_new(&ODBCG(connections), hashed_details, hashed_details_len, return_value);
2338+
23362339
ODBCG(num_persistent)++;
23372340
ODBCG(num_links)++;
23382341
} else { /* found connection */
@@ -2381,15 +2384,22 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
23812384
}
23822385
}
23832386

2384-
object_init_ex(return_value, odbc_connection_ce);
2385-
odbc_link *link = Z_ODBC_LINK_P(return_value);
2386-
link->connection = db_conn;
2387-
link->hash = zend_string_init(hashed_details, hashed_details_len, persistent);
2388-
link->persistent = true;
2387+
zval *tmp;
2388+
if ((tmp = zend_hash_str_find(&ODBCG(connections), hashed_details, hashed_details_len)) == NULL) {
2389+
object_init_ex(return_value, odbc_connection_ce);
2390+
odbc_link *link = Z_ODBC_LINK_P(return_value);
2391+
link->connection = db_conn;
2392+
link->hash = zend_string_init(hashed_details, hashed_details_len, persistent);
2393+
link->persistent = true;
2394+
} else {
2395+
ZVAL_COPY(return_value, tmp);
2396+
2397+
ZEND_ASSERT(Z_ODBC_CONNECTION_P(return_value) == db_conn && "Persistent connection has changed");
2398+
}
23892399
}
2390-
} else {
2400+
} else { /* non-persistent */
23912401
zval *tmp;
2392-
if ((tmp = zend_hash_str_find(&ODBCG(non_persistent_connections), hashed_details, hashed_details_len)) == NULL) { /* non-persistent, new */
2402+
if ((tmp = zend_hash_str_find(&ODBCG(connections), hashed_details, hashed_details_len)) == NULL) { /* non-persistent, new */
23932403
if (ODBCG(max_links) != -1 && ODBCG(num_links) >= ODBCG(max_links)) {
23942404
php_error_docref(NULL, E_WARNING, "Too many open connections (" ZEND_LONG_FMT ")", ODBCG(num_links));
23952405
efree(hashed_details);
@@ -2403,7 +2413,7 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
24032413
}
24042414
ODBCG(num_links)++;
24052415

2406-
zend_hash_str_add_new(&ODBCG(non_persistent_connections), hashed_details, hashed_details_len, return_value);
2416+
zend_hash_str_add_new(&ODBCG(connections), hashed_details, hashed_details_len, return_value);
24072417
} else { /* non-persistent, pre-existing */
24082418
ZVAL_COPY(return_value, tmp);
24092419
}
@@ -2417,19 +2427,21 @@ PHP_FUNCTION(odbc_close)
24172427
{
24182428
zval *pv_conn;
24192429
odbc_link *link;
2430+
odbc_connection *connection;
24202431

24212432
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &pv_conn, odbc_connection_ce) == FAILURE) {
24222433
RETURN_THROWS();
24232434
}
24242435

24252436
link = Z_ODBC_LINK_P(pv_conn);
2426-
CHECK_ODBC_CONNECTION(link->connection);
2437+
connection = Z_ODBC_CONNECTION_P(pv_conn);
2438+
CHECK_ODBC_CONNECTION(connection);
2439+
2440+
odbc_link_free(link);
24272441

24282442
if (link->persistent) {
2429-
zend_hash_apply_with_argument(&EG(persistent_list), _close_pconn_with_res, (void *) Z_OBJ_P(pv_conn));
2443+
zend_hash_apply_with_argument(&EG(persistent_list), _close_pconn_with_res, (void *) connection);
24302444
}
2431-
2432-
odbc_link_free(link);
24332445
}
24342446
/* }}} */
24352447

‎ext/odbc/php_odbc_includes.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,13 @@ ZEND_BEGIN_MODULE_GLOBALS(odbc)
248248
zend_long default_cursortype;
249249
char laststate[6];
250250
char lasterrormsg[SQL_MAX_MESSAGE_LENGTH];
251-
HashTable non_persistent_connections;
251+
/* Stores ODBC links throughout the duration of a request. The connection member may be either persistent or
252+
* non-persistent. In the former case, it is a pointer to an item in EG(persistent_list). This solution makes it
253+
* possible to properly free links during RSHUTDOWN (or when they are explicitly closed), while persistent
254+
* connections themselves are going to be freed later during the shutdown process (or when they are explicitly
255+
* closed).
256+
*/
257+
HashTable connections;
252258
ZEND_END_MODULE_GLOBALS(odbc)
253259

254260
int odbc_add_result(HashTable *list, odbc_result *result);

‎ext/odbc/tests/odbc_close_001.phpt‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
--TEST--
2+
odbc_close(): Basic test
3+
--EXTENSIONS--
4+
odbc
5+
--SKIPIF--
6+
<?php include 'skipif.inc'; ?>
7+
--FILE--
8+
<?php
9+
10+
include 'config.inc';
11+
12+
$conn1 = odbc_connect($dsn, $user, $pass);
13+
$conn2 = odbc_pconnect($dsn, $user, $pass);
14+
$result1 = odbc_columns($conn1, '', '', '', '');
15+
$result2 = odbc_columns($conn2, '', '', '', '');
16+
17+
var_dump($conn1);
18+
var_dump($conn2);
19+
var_dump($result1);
20+
var_dump($result2);
21+
22+
odbc_close($conn1);
23+
odbc_close($conn2);
24+
25+
try {
26+
odbc_columns($conn1, '', '', '', '');
27+
} catch (Error $e) {
28+
echo $e->getMessage() . "\n";
29+
}
30+
31+
try {
32+
odbc_columns($conn2, '', '', '', '');
33+
} catch (Error $e) {
34+
echo $e->getMessage() . "\n";
35+
}
36+
37+
try {
38+
odbc_num_rows($result1);
39+
} catch (Error $e) {
40+
echo $e->getMessage() . "\n";
41+
}
42+
43+
try {
44+
odbc_num_rows($result2);
45+
} catch (Error $e) {
46+
echo $e->getMessage() . "\n";
47+
}
48+
49+
?>
50+
--EXPECTF--
51+
object(ODBC\Connection)#%d (%d) {
52+
}
53+
object(ODBC\Connection)#%d (%d) {
54+
}
55+
object(ODBC\Result)#%d (%d) {
56+
}
57+
object(ODBC\Result)#%d (%d) {
58+
}
59+
ODBC connection has already been closed
60+
ODBC connection has already been closed
61+
ODBC result has already been closed
62+
ODBC result has already been closed

‎ext/odbc/tests/odbc_close_all_001.phpt‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,32 @@ var_dump($result2);
2121

2222
odbc_close_all();
2323

24-
var_dump($conn1);
25-
var_dump($conn2);
26-
var_dump($result1);
27-
var_dump($result2);
28-
29-
?>
30-
--EXPECTF--
31-
object(ODBC\Connection)#%d (%d) {
24+
try {
25+
odbc_columns($conn1, '', '', '', '');
26+
} catch (Error $e) {
27+
echo $e->getMessage() . "\n";
3228
}
33-
object(ODBC\Connection)#%d (%d) {
29+
30+
try {
31+
odbc_columns($conn2, '', '', '', '');
32+
} catch (Error $e) {
33+
echo $e->getMessage() . "\n";
3434
}
35-
object(ODBC\Result)#%d (%d) {
35+
36+
try {
37+
odbc_num_rows($result1);
38+
} catch (Error $e) {
39+
echo $e->getMessage() . "\n";
3640
}
37-
object(ODBC\Result)#%d (%d) {
41+
42+
try {
43+
odbc_num_rows($result2);
44+
} catch (Error $e) {
45+
echo $e->getMessage() . "\n";
3846
}
47+
48+
?>
49+
--EXPECTF--
3950
object(ODBC\Connection)#%d (%d) {
4051
}
4152
object(ODBC\Connection)#%d (%d) {
@@ -44,3 +55,7 @@ object(ODBC\Result)#%d (%d) {
4455
}
4556
object(ODBC\Result)#%d (%d) {
4657
}
58+
ODBC connection has already been closed
59+
ODBC connection has already been closed
60+
ODBC result has already been closed
61+
ODBC result has already been closed

0 commit comments

Comments
 (0)