Skip to content

lightningd/channel.c: Transfer peer to connectd only if connectd alive. #2694

Merged
cdecker merged 2 commits into
ElementsProject:masterfrom
ZmnSCPxj:fix-2677
May 31, 2019
Merged

lightningd/channel.c: Transfer peer to connectd only if connectd alive. #2694
cdecker merged 2 commits into
ElementsProject:masterfrom
ZmnSCPxj:fix-2677

Conversation

@ZmnSCPxj
Copy link
Copy Markdown
Contributor

Fixes #2677

I am unable to replicate 2677 in a test; it seems to me that there is something that prevents subdaemons of lightningd to respond quickly to communications with lightningd, but it is not present in our CI.

@m-schmoock
Copy link
Copy Markdown
Collaborator

m-schmoock commented May 31, 2019

Yes, I have this on both of my Archlinux machines. This PR fixes the memory corruptions. As you can see in the timestamps my logs, the subdeamons take quite some time to shut down:

2019-05-31T09:38:26.700Z lightningd(5024):jcon fd 33: JSON-RPC shutdown
2019-05-31T09:38:36.702Z lightning_connectd(5038): Status closed, but waitpid 5038 says No child processes
2019-05-31T09:38:46.704Z lightning_gossipd(5039): Status closed, but waitpid 5039 says No child processes
2019-05-31T09:38:56.707Z lightning_hsmd(5037): Status closed, but waitpid 5037 says No child processes

Also valgrind seems to be happier now:

==24884== HEAP SUMMARY:
==24884==     in use at exit: 140,706 bytes in 80 blocks
==24884==   total heap usage: 509,154 allocs, 509,074 frees, 105,248,624 bytes allocated
==24884== 
==24884== LEAK SUMMARY:
==24884==    definitely lost: 137,264 bytes in 16 blocks
==24884==    indirectly lost: 2,304 bytes in 48 blocks
==24884==      possibly lost: 0 bytes in 0 blocks
==24884==    still reachable: 1,138 bytes in 16 blocks
==24884==         suppressed: 0 bytes in 0 blocks
==24884== Rerun with --leak-check=full to see details of leaked memory
==24884== 
==24884== For counts of detected and suppressed errors, rerun with: -v
==24884== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Copy link
Copy Markdown
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

ACK 64b9517

@cdecker cdecker merged commit 3466261 into ElementsProject:master May 31, 2019
@ZmnSCPxj ZmnSCPxj deleted the fix-2677 branch May 31, 2019 14:19
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.

Crash: When shutting down the daemon

3 participants