Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory leaks from "Maybe_Unicode" (fixes #27 #125) #145

Merged
merged 1 commit into from Mar 28, 2020

Conversation

henryykt
Copy link
Contributor

Attempt to fix memory leaks described in #27 and #125.

@auvipy auvipy requested a review from matusvalo March 23, 2020 13:40
@thedrow
Copy link
Member

thedrow commented Mar 23, 2020

Did you verify this patch with valgrind?

@henryykt
Copy link
Contributor Author

Did you verify this patch with valgrind?

Tested with the following:

import json

from librabbitmq import Connection, Message

max_iters = 10000

with Connection(
    host="localhost", port=5672, userid="guest", password="guest", virtual_host="/"
) as conn:
    with conn.channel() as channel:
        channel.exchange_declare(exchange="re", type="direct", auto_delete=False)

        data = json.dumps({"some_key": "some_value"})
        props = {"headers": {"some_header": "some_header_value"}}

        while max_iters > 0:
            max_iters -= 1

            message = Message(channel=channel, properties=props, body=data)
            channel.basic_publish(message, 're', routing_key='')

Output master branch:

(venv_librabbitmq) [developer@dev librabbitmq]$ PYTHONMALLOC=malloc valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all python ./publish.py
==26039== Memcheck, a memory error detector
==26039== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==26039== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==26039== Command: python ./publish.py
==26039==
...
==26039== LEAK SUMMARY:
==26039==    definitely lost: 1,289,804 bytes in 29,996 blocks
==26039==    indirectly lost: 0 bytes in 0 blocks
==26039==      possibly lost: 641,114 bytes in 3,902 blocks
==26039==    still reachable: 1,428,011 bytes in 14,090 blocks
==26039==                       of which reachable via heuristic:
==26039==                         newarray           : 60 bytes in 1 blocks
==26039==         suppressed: 0 bytes in 0 blocks
==26039==
==26039== For counts of detected and suppressed errors, rerun with: -v
==26039== ERROR SUMMARY: 717 errors from 717 contexts (suppressed: 0 from 0)

Output with fix:

(venv_librabbitmq) [developer@dev librabbitmq]$ PYTHONMALLOC=malloc valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all python ./publish.py
==26385== Memcheck, a memory error detector
==26385== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==26385== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==26385== Command: python ./publish.py
==26385==
...
==26385== LEAK SUMMARY:
==26385==    definitely lost: 0 bytes in 0 blocks
==26385==    indirectly lost: 0 bytes in 0 blocks
==26385==      possibly lost: 640,888 bytes in 3,897 blocks
==26385==    still reachable: 1,427,967 bytes in 14,089 blocks
==26385==                       of which reachable via heuristic:
==26385==                         newarray           : 60 bytes in 1 blocks
==26385==         suppressed: 0 bytes in 0 blocks
==26385==
==26385== For counts of detected and suppressed errors, rerun with: -v
==26385== ERROR SUMMARY: 710 errors from 710 contexts (suppressed: 0 from 0)

Copy link
Member

@thedrow thedrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@auvipy
Copy link
Member

auvipy commented Mar 28, 2020

can you rebase?

@henryykt
Copy link
Contributor Author

can you rebase?

done

@auvipy auvipy merged commit 13a49b0 into celery:master Mar 28, 2020
@henryykt henryykt deleted the fix_mememory_leak branch March 28, 2020 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants