Обсуждение: Help needed for the resolution of memory leak

Thanks Merlin,

will try to check in other places as well and get back in case any further questions

Regards,

Sasmit Utkarsh

+91-7674022625

On Tue, Jan 16, 2024 at 1:30 AM Merlin Moncure <mmoncure@gmail.com> wrote:

On Mon, Jan 15, 2024 at 11:32 AM Sasmit Utkarsh <utkarshsasmit@gmail.com> wrote:

Hi Team,

I am trying to test a code which basically tries to read some data from postgresql db in a loop through a function SQL_get_tpf_rw() whose definition and other details are shared in the attached file along with the memory leak report resulted during testing. Could you let me know if i missed calling  anywhere PQclear()

in SQL_get_tpf_rw() which resulted in the below .

Last few lines from the report

yep:  for example,

      if (PQresultStatus(res) != PGRES_TUPLES_OK)           {                LOG_ERROR("SELECT failed: %s", PQerrorMessage(conn));		LOG_DEBUG("ROLLBACK TRANSACTION AND CHAIN");		res = PQexec(conn,"ROLLBACK TRANSACTION AND CHAIN");		LOG_ERROR("INVALID_FILE_ADDRESS %08X",fa);		rc = ERR_INVALID_FILE_ADDRESS;

See highlighted bits.  You're reusing the res object before clearing it.  there may be other cases, but this jumped off the page.

merlin

On Tue, Jan 16, 2024 at 9:10 AM Sasmit Utkarsh <utkarshsasmit@gmail.com> wrote:

Hi Merlin et al.

I have tried to have the above change added in the missing places. Still, I see the below leaks reported by the address sanitizer. Please see the attachments for the leak reported and the function definition updated.  Not sure for PQexecPrepared if we call PQclear(res) for cleaning up as well. let me know if you need any more information

You have many other leaks.    See code below, you are making up to thee sequential calls to create a result before calling clear.  All calls creating and returning returning PGresult have to be cleared before the pointer is reused.

merlin

	LOG_DEBUG("%s() conninfo=%s",__func__,conninfo);	
	if(is_shadow_db)	{		shadow_db_conn = PQconnectdb(shadow_db_conn_info);		if ( PQstatus(shadow_db_conn ) != CONNECTION_OK )		{		   LOG_ERROR("Connection to shadow database failed! %s", PQerrorMessage(conn));		   PQfinish(shadow_db_conn);		   exit(1);		}		res = PQexec(shadow_db_conn, "SET bytea_output = 'escape'");		LOG_DEBUG("%s() Connection to shadow_shc_data database SUCCESSFUL",__func__);		// execute_stored_procedure(shadow_db_conn);	}
	conn = PQconnectdb(conninfo);	if ( PQstatus(conn) != CONNECTION_OK ) {		LOG_ERROR("Connection to database failed! %s", PQerrorMessage(conn));		PQfinish(conn);		exit(1);	} else {		res = PQexec(conn, "SET bytea_output = 'escape'");		LOG_DEBUG("%s() Connection to shc_data database SUCCESSFUL",__func__);	}
	res = PQexec(conn, "START TRANSACTION");	if (PQresultStatus(res) != PGRES_COMMAND_OK)	{		LOG_ERROR("START TRANSACTION failed: %s", PQerrorMessage(conn));		SQL_exit_nicely(conn,res);	}	PQclear(res);

Thanks, I'll take a look. 

Regards,

Sasmit Utkarsh

+91-7674022625

On Wed, Jan 17, 2024 at 4:12 AM Merlin Moncure <mmoncure@gmail.com> wrote:

On Tue, Jan 16, 2024 at 9:10 AM Sasmit Utkarsh <utkarshsasmit@gmail.com> wrote:

Hi Merlin et al.

I have tried to have the above change added in the missing places. Still, I see the below leaks reported by the address sanitizer. Please see the attachments for the leak reported and the function definition updated.  Not sure for PQexecPrepared if we call PQclear(res) for cleaning up as well. let me know if you need any more information

You have many other leaks.    See code below, you are making up to thee sequential calls to create a result before calling clear.  All calls creating and returning returning PGresult have to be cleared before the pointer is reused.

merlin

	LOG_DEBUG("%s() conninfo=%s",__func__,conninfo);	
	if(is_shadow_db)	{		shadow_db_conn = PQconnectdb(shadow_db_conn_info);		if ( PQstatus(shadow_db_conn ) != CONNECTION_OK )		{		   LOG_ERROR("Connection to shadow database failed! %s", PQerrorMessage(conn));		   PQfinish(shadow_db_conn);		   exit(1);		}		res = PQexec(shadow_db_conn, "SET bytea_output = 'escape'");		LOG_DEBUG("%s() Connection to shadow_shc_data database SUCCESSFUL",__func__);		// execute_stored_procedure(shadow_db_conn);	}
	conn = PQconnectdb(conninfo);	if ( PQstatus(conn) != CONNECTION_OK ) {		LOG_ERROR("Connection to database failed! %s", PQerrorMessage(conn));		PQfinish(conn);		exit(1);	} else {		res = PQexec(conn, "SET bytea_output = 'escape'");		LOG_DEBUG("%s() Connection to shc_data database SUCCESSFUL",__func__);	}
	res = PQexec(conn, "START TRANSACTION");	if (PQresultStatus(res) != PGRES_COMMAND_OK)	{		LOG_ERROR("START TRANSACTION failed: %s", PQerrorMessage(conn));		SQL_exit_nicely(conn,res);	}	PQclear(res);

On Wed, Jan 17, 2024 at 1:14 PM Sasmit Utkarsh <utkarshsasmit@gmail.com> wrote:

Hi Merlin et al.

Below are some couple of observations attached as an "overview_of_code" and other attachments "function_def_other_details" and leak sanitizer report. Please assist with some clarifications given in overview_of_code with (***). Let me know if you need any more information

***How do we handle for the case clearing when PGresult object is assigned a pointer to the data of the specified field within the existing PGresult object?
i.e when SQL_get_tpf_rw() actually completes in each iteration? 

It is your responsibility to close PGResult and PGConn objects.  Each one created must be cleaned up.  This is basic libpq usage.  I suggest studying the documentation.  

Start here: https://www.postgresql.org/docs/current/libpq-exec.html 
Also Study here: https://www.postgresql.org/docs/current/libpq-example.html

You should not reuse a pointer unless you have cleared the object first.

****Is the leak reported due to improper handling of the above case ? or is it due to some other flow

Your leaks look mostly due to not cleaning PGResult.  However, the real issue here is you need to learn basic libpq usage a little better...try writing a smaller program and see when it starts to complain about leaks.

merlin