Thread: 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

        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?

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.

>
>>
>>
>> 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.
>

And the error case was handled correctly, right?

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
lookslike.  Though, I’d probably use PQresultErrorMessage and check affirmatively for the tuples and error cases and
havea 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.

>
> 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

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

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.

>
>>
>>
>> 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.
>