atsign-foundation / noports

Connect to any device with no external listening ports open
https://noports.com
BSD 3-Clause "New" or "Revised" License
267 stars 15 forks source link

fix: (c sshnpd) allocate threaded memory outside of the thread #1171

Closed JeremyTubongbanua closed 3 months ago

JeremyTubongbanua commented 3 months ago

- What I did

- How to verify it

new leak summary (after closing thread)

notice that the 585 bytes that were usually suppressed are no longer lost.

==28467== LEAK SUMMARY:
==28467==    definitely lost: 0 bytes in 0 blocks
==28467==    indirectly lost: 0 bytes in 0 blocks
==28467==      possibly lost: 272 bytes in 1 blocks
==28467==    still reachable: 121,622 bytes in 201 blocks
==28467==         suppressed: 0 bytes in 0 blocks

- Description for the changelog fix: no longer lose 585B every time SSH session is established

JeremyTubongbanua commented 3 months ago

running into a 16 KB leak after establishing SSH connection, converting to draft while I investigate

JeremyTubongbanua commented 3 months ago

After establishing an SSH session with 2 fresh atSigns, we get 16KB lost.

Threaded leak summary

==68541== LEAK SUMMARY:
==68541==    definitely lost: 16,384 bytes in 2 blocks
==68541==    indirectly lost: 0 bytes in 0 blocks
==68541==      possibly lost: 272 bytes in 1 blocks
==68541==    still reachable: 121,622 bytes in 201 blocks
==68541==         suppressed: 0 bytes in 0 blocks

Main process leak summary

==68485== LEAK SUMMARY:
==68485==    definitely lost: 16,384 bytes in 2 blocks
==68485==    indirectly lost: 0 bytes in 0 blocks
==68485==      possibly lost: 0 bytes in 0 blocks
==68485==    still reachable: 0 bytes in 0 blocks
==68485==         suppressed: 0 bytes in 0 blocks

Full valgrind logs: https://gist.github.com/JeremyTubongbanua/a49a783ea400506d871239873224d1a8

JeremyTubongbanua commented 3 months ago

After establishing an SSH session with 2 fresh atSigns, we get 16KB lost.

Threaded leak summary

==68541== LEAK SUMMARY:
==68541==    definitely lost: 16,384 bytes in 2 blocks
==68541==    indirectly lost: 0 bytes in 0 blocks
==68541==      possibly lost: 272 bytes in 1 blocks
==68541==    still reachable: 121,622 bytes in 201 blocks
==68541==         suppressed: 0 bytes in 0 blocks

Main process leak summary

==68485== LEAK SUMMARY:
==68485==    definitely lost: 16,384 bytes in 2 blocks
==68485==    indirectly lost: 0 bytes in 0 blocks
==68485==      possibly lost: 0 bytes in 0 blocks
==68485==    still reachable: 0 bytes in 0 blocks
==68485==         suppressed: 0 bytes in 0 blocks

Full valgrind logs: https://gist.github.com/JeremyTubongbanua/a49a783ea400506d871239873224d1a8

The reason for this leak is at_c related. See https://github.com/atsign-foundation/at_c/pull/328 for an upcoming fix.

Next commit will most likely be a commit hash change for atsdk so that the C Daemon will use the latest trunk hash for the memory leak fix.