Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

Commit

Permalink
modify db connection error handling in the ido2db child to prevent en…
Browse files Browse the repository at this point in the history 
…dless forks #2458

when the handle_client_connection function is being run, reading from
the socket, and then passing that to a dynamic buffer, the
check_for_client_input call will happen, and then checking if the
idodmod client should be disconnected due to the gotten data.
within handle_client_input, it will be made clear that the initial
db_hello needs to take place. problem at this stage is, that a
non-existing database connection will cause this function to fail,
calling its own wipe threads/memory/connection functionality plus a hard
call to _exit(0) which will leave the socket unclosed and open - and
therefore leading to a new handshake with idomod on the next loop run.

within db_hello, the db_version_check takes place as well, where errors
are ignored (cause return value not taken into account) plus the
instance name check depends on a valid db connection, which will lead
into an error on db_query call (where a call to db_reconnect happens,
which will fail too). the handle_db_error function will now set
disconnect_client=IDO_TRUE, but after not fetching the correct values,
the else condition will match, trying to hard-exit the child in process.
this exit will lead into the mentioned leftopen socket, leading to
multiple forks on new connections - while the childs are left in defunct
or socket_wait, as their file descriptor is still open on the socket -
which is still in use by the first and following n forks. so this is a
valid race condition amongst the first false exit leaving the socket
open, as well as the others not getting rid of it after their exit calls
as well.

the proper fix for this brainfuck is merely to check the return values
of each function call in this row, and bailing early without wiping
anything nor exiting the child. this is being left alone to the
handle_client_connection function, which will normally decide upon
disconnect_client==IDO_TRUE? if it is time to disconnect idomod AND
close the socket. plus terminate the threads and clean the memory.

same goes for the housekeeping thread as well, this is not even allowed
to call _exit(0) on its main process, and therefore will return an error
on db connection failure too, whereas this thread connection polls in
5sec interval, if a valid connection and/or instance name is available.

overall, this is not the best fix, but it is more clean than closing the
socket before invoking an uncontrolled call to _exit(0) - which probably
would have saved the hassle as well - did not try though.

refs #2458
  • Loading branch information
Michael Friedrich committed May 14, 2012
1 parent 311093a commit 4c9b080
Showing 1 changed file with 27 additions and 74 deletions.
101 changes: 27 additions & 74 deletions module/idoutils/src/db.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,8 @@ int ido2db_db_is_connected(ido2db_idi *idi) {

int ido2db_db_reconnect(ido2db_idi *idi) {

int result = IDO_OK;

/* check connection */
if (ido2db_db_is_connected(idi) == IDO_FALSE)
idi->dbinfo.connected = IDO_FALSE;
Expand All @@ -411,10 +413,10 @@ int ido2db_db_reconnect(ido2db_idi *idi) {
syslog(LOG_USER | LOG_INFO, "Error: Could not reconnect to database!");
return IDO_ERROR;
}
ido2db_db_hello(idi);
result = ido2db_db_hello(idi);
}

return IDO_OK;
return result;
}


Expand Down Expand Up @@ -1317,6 +1319,8 @@ int ido2db_db_version_check(ido2db_idi *idi) {
void *data[1];
buf = NULL;
name = NULL;
int result = IDO_OK;

ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check () start \n");

name = strdup("idoutils");
Expand All @@ -1327,7 +1331,7 @@ int ido2db_db_version_check(ido2db_idi *idi) {
if (asprintf(&buf, "SELECT version FROM %s WHERE name='%s'", ido2db_db_tablenames[IDO2DB_DBTABLE_DBVERSION], name) == -1)
buf = NULL;

if ((ido2db_db_query(idi, buf)) == IDO_OK) {
if ((result = ido2db_db_query(idi, buf)) == IDO_OK) {

if (idi->dbinfo.dbi_result != NULL) {
if (dbi_result_next_row(idi->dbinfo.dbi_result)) {
Expand Down Expand Up @@ -1363,7 +1367,7 @@ int ido2db_db_version_check(ido2db_idi *idi) {
if (OCI_FetchNext(idi->dbinfo.oci_resultset)) {
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check() fetchnext ok\n");
idi->dbinfo.dbversion = strdup(OCI_GetString(idi->dbinfo.oci_resultset, 1));
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_hello(version=%s)\n", idi->dbinfo.dbversion);
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check(version=%s)\n", idi->dbinfo.dbversion);
} else {
idi->dbinfo.dbversion = NULL;
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check() fetchnext not ok\n");
Expand Down Expand Up @@ -1393,7 +1397,7 @@ int ido2db_db_version_check(ido2db_idi *idi) {

ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check () end\n");

return IDO_OK;
return result;

}

Expand All @@ -1418,7 +1422,16 @@ int ido2db_db_hello(ido2db_idi *idi) {
idi->instance_name = strdup("default");


ido2db_db_version_check(idi);
result = ido2db_db_version_check(idi);

if (result == IDO_ERROR) {
syslog(LOG_USER | LOG_INFO, "Error: DB Version Check against %s database query failed! Please check %s database configuration and schema!", ido2db_db_settings.dbserver, ido2db_db_settings.dbserver);
syslog(LOG_USER | LOG_INFO, "Exiting ...");

ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_version_check() query against existing instance not possible, cleaning up and exiting\n");

return IDO_ERROR;
}

#ifdef USE_LIBDBI /* everything else will be libdbi */

Expand All @@ -1445,23 +1458,8 @@ int ido2db_db_hello(ido2db_idi *idi) {

ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_hello() query against existing instance not possible, cleaning up and exiting\n");

ido2db_terminate_threads();

/* disconnect from database */
ido2db_db_disconnect(idi);
ido2db_db_deinit(idi);

/* free memory */
ido2db_free_input_memory(idi);
ido2db_free_connection_memory(idi);

/* cleanup the socket */
ido2db_cleanup_socket();

/* free memory */
ido2db_free_program_memory();

_exit(0);
/* bail out, but do not exit the child yet */
return IDO_ERROR;
}

dbi_result_free(idi->dbinfo.dbi_result);
Expand Down Expand Up @@ -1493,23 +1491,8 @@ int ido2db_db_hello(ido2db_idi *idi) {

ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_db_hello() query against existing instance not possible, cleaning up and exiting\n");

ido2db_terminate_threads();

/* disconnect from database */
ido2db_db_disconnect(idi);
ido2db_db_deinit(idi);

/* free memory */
ido2db_free_input_memory(idi);
ido2db_free_connection_memory(idi);

/* cleanup the socket */
ido2db_cleanup_socket();

/* free memory */
ido2db_free_program_memory();

_exit(0);
/* bail out, but do not exit the child yet */
return IDO_ERROR;
}

/* commit statement */
Expand Down Expand Up @@ -1892,23 +1875,8 @@ int ido2db_thread_db_hello(ido2db_idi *idi) {
} else {
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_thread_db_hello() query against existing instance not possible, cleaning up and exiting\n");

ido2db_terminate_threads();

/* disconnect from database */
ido2db_db_disconnect(idi);
ido2db_db_deinit(idi);

/* free memory */
ido2db_free_input_memory(idi);
ido2db_free_connection_memory(idi);

/* cleanup the socket */
ido2db_cleanup_socket();

/* free memory */
ido2db_free_program_memory();

_exit(0);
/* bail out, but do not exit the child yet - not allowed as thread */
return IDO_ERROR;
}

dbi_result_free(idi->dbinfo.dbi_result);
Expand All @@ -1933,23 +1901,8 @@ int ido2db_thread_db_hello(ido2db_idi *idi) {
if (!OCI_Execute(idi->dbinfo.oci_statement_instances_select)) {
ido2db_log_debug_info(IDO2DB_DEBUGL_PROCESSINFO, 2, "ido2db_thread_db_hello() query against existing instance not possible, cleaning up and exiting\n");

ido2db_terminate_threads();

/* disconnect from database */
ido2db_db_disconnect(idi);
ido2db_db_deinit(idi);

/* free memory */
ido2db_free_input_memory(idi);
ido2db_free_connection_memory(idi);

/* cleanup the socket */
ido2db_cleanup_socket();

/* free memory */
ido2db_free_program_memory();

_exit(0);
/* bail out, but do not exit the child yet - not allowed as thread */
return IDO_ERROR;
}

/* commit statement */
Expand Down

0 comments on commit 4c9b080

Please sign in to comment.