How to properly fix memory leak
Hi, ALL,
[code]
auto res = PQexec( m_db, m_pimpl->m_myconv.to_bytes( query.c_str()
).c_str() ); /* ask for binary results */
if( PQresultStatus( res ) != PGRES_TUPLES_OK )
{
auto err = m_pimpl->m_myconv.from_bytes( PQerrorMessage( m_db ) );
errorMsg.push_back( L"Update validation table: " + err );
result = 1;
}
else
{
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]
I ran this code on MSVC 2017 with VLD and according to the VLD report I have
a memory leak on the line indicated.
Should I call PQclear() on every iteration of the loop?
And I hope I handle the error cae properly...
Thank you
On Friday, April 25, 2025, Igor Korot <ikorot01@gmail.com> wrote:
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]I ran this code on MSVC 2017 with VLD and according to the VLD report I
have
a memory leak on the line indicated.
Seems like a false positive.
Should I call PQclear() on every iteration of the loop?
Would make processing more than a single row impossible if you throw away
the result after processing one row.
David J.
Hi, David,
On Fri, Apr 25, 2025 at 10:48 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
On Friday, April 25, 2025, Igor Korot <ikorot01@gmail.com> wrote:
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]I ran this code on MSVC 2017 with VLD and according to the VLD report I have
a memory leak on the line indicated.Seems like a false positive.
And the error case was handled correctly, right?
Thank you.
Show quoted text
Should I call PQclear() on every iteration of the loop?
Would make processing more than a single row impossible if you throw away the result after processing one row.
David J.
On Friday, April 25, 2025, Igor Korot <ikorot01@gmail.com> wrote:
And the error case was handled correctly, right?
Seems like answering that requires knowing what the query is or can be. I
also have no idea what idiomatic code looks like. Though, I’d probably use
PQresultErrorMessage and check affirmatively for the tuples and error cases
and have a final else should the status be something unexpected.
David J.
Hi, David,
On Fri, Apr 25, 2025 at 11:55 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
On Friday, April 25, 2025, Igor Korot <ikorot01@gmail.com> wrote:
And the error case was handled correctly, right?
Seems like answering that requires knowing what the query is or can be. I also have no idea what idiomatic code looks like. Though, I’d probably use PQresultErrorMessage and check affirmatively for the tuples and error cases and have a final else should the status be something unexpected.
Understood.
Below is the full function:
[code]
int PostgresDatabase::PopulateTablespaces(std::vector<std::wstring> &errorMsg)
{
int result = 0;
std::wstring errorMessage;
std::wstring query = L"SELECT * FROM pg_tablespace;";
auto res = PQexec( m_db, m_pimpl->m_myconv.to_bytes( query.c_str()
).c_str() ); /* ask for binary results */
if( PQresultStatus( res ) != PGRES_TUPLES_OK )
{
auto err = m_pimpl->m_myconv.from_bytes( PQerrorMessage( m_db ) );
errorMsg.push_back( L"Update validation table: " + err );
result = 1;
}
else
{
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
}
}
PQclear( res );
return result;
}
[/code]
Thank you.
Show quoted text
David J.
On Fri, 2025-04-25 at 22:24 -0500, Igor Korot wrote:
Hi, ALL,
[code]
auto res = PQexec( m_db, m_pimpl->m_myconv.to_bytes( query.c_str()
).c_str() ); /* ask for binary results */
if( PQresultStatus( res ) != PGRES_TUPLES_OK )
{
auto err = m_pimpl->m_myconv.from_bytes( PQerrorMessage( m_db ) );
errorMsg.push_back( L"Update validation table: " + err );
result = 1;
}
else
{
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]I ran this code on MSVC 2017 with VLD and according to the VLD report I have
a memory leak on the line indicated.Should I call PQclear() on every iteration of the loop?
And I hope I handle the error cae properly...
No, PQclear() would cause an error (double free).
If it is not a spurious complaint, the leak would have to be in m_tablespaces.push_back().
Yours,
Laurenz Albe
Hi, Lauren’s,
On Sat, Apr 26, 2025 at 3:30 PM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:
On Fri, 2025-04-25 at 22:24 -0500, Igor Korot wrote:
Hi, ALL,
[code]
auto res = PQexec( m_db, m_pimpl->m_myconv.to_bytes( query.c_str()
).c_str() ); /* ask for binary results */
if( PQresultStatus( res ) != PGRES_TUPLES_OK )
{
auto err = m_pimpl->m_myconv.from_bytes( PQerrorMessage( m_db ));
errorMsg.push_back( L"Update validation table: " + err );
result = 1;
}
else
{
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]I ran this code on MSVC 2017 with VLD and according to the VLD report I
have
a memory leak on the line indicated.
Should I call PQclear() on every iteration of the loop?
And I hope I handle the error cae properly...
No, PQclear() would cause an error (double free).
If it is not a spurious complaint, the leak would have to be in
m_tablespaces.push_back().
No, it is not spurious.
I’m getting it every time I run the program.
The m_tablespaces variable is declared as “std::vector<std::wstring>. No
pointer is involved.
I don’t see how it can produce the leak…
Thank you.
Show quoted text
Yours,
Laurenz Albe
Igor Korot <ikorot01@gmail.com> writes:
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
I would imagine that from_bytes() is producing a malloc'd
string. Which part of this is responsible for seeing that
that gets freed?
regards, tom lane
Hi, Tom,
On Sat, Apr 26, 2025 at 4:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Igor Korot <ikorot01@gmail.com> writes:
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );I would imagine that from_bytes() is producing a malloc'd
string. Which part of this is responsible for seeing that
that gets freed?
No other places produces memory leak.
And I use this function quite extensively…
Thank you.
Show quoted text
regards, tom lane
David et al,
On Fri, Apr 25, 2025 at 10:48 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
On Friday, April 25, 2025, Igor Korot <ikorot01@gmail.com> wrote:
for( int i = 0; i < PQntuples( res ); i++ )
{
auto temp1 = m_pimpl->m_myconv.from_bytes( PQgetvalue(
res, i, 1 ) );
m_tablespaces.push_back( temp1 );
} // this line gives a leak according to VLD
}
PQclear( res );
return result;
[/code]I ran this code on MSVC 2017 with VLD and according to the VLD report I have
a memory leak on the line indicated.Seems like a false positive.
Looks like it is false positive.
I ran the code under valgrind and there I didn't get
such a leak.
I did however get a different issues which I fixed.
But even after moving the fixes ober to Windows and trying
to run it - I still see that.
For now I put this to bed as it is not an issue on Linux.
Thank you.
Show quoted text
Should I call PQclear() on every iteration of the loop?
Would make processing more than a single row impossible if you throw away the result after processing one row.
David J.