fix: Fix shard reconnection issue causing segfault by igor725 · Pull Request #1608 · brainboxdotcc/DPP

igor725 · 2026-07-07T15:16:27Z

This should fix the old shard is not being there if something in network stack has failed during the reconnection stage. Additionally I removed what looks to be a pair of duplicate parameters with unnecessary std::string copying in the reconnect for client constructor, since old is being deleted long after new discord_client being constructed, it shouldn't be a problem.

Here's traces from my reconnect test:

[2026-07-07 18:06:44] TRACE: Shard 0 not ready to reconnect yet.
[2026-07-07 18:06:44] TRACE: Shard 0 not ready to reconnect yet.
[2026-07-07 18:06:44] TRACE: Shard 0 not ready to reconnect yet.
[2026-07-07 18:06:44] TRACE: Shard 0 not ready to reconnect yet.
[2026-07-07 18:06:45] INFO: Reconnecting shard 0
[2026-07-07 18:06:45] TRACE: Attempting resume...
[2026-07-07 18:06:45] TRACE: Running new connection...
[2026-07-07 18:06:45] TRACE: Deleting old connection...
[2026-07-07 18:06:45] TRACE: Installing new connection...

It's not the actual DNS failure test because it's kinda hard to catch the thing in action, but at least I proved that reconnection mechanism is not broken, my bot still keeps replying to messages after the reconnection.

Code change checklist

I have ensured that all methods and functions are fully documented using doxygen style comments.
My code follows the coding style guide.
I tested that my change works before raising the PR.
I have ensured that I did not break any existing API calls.
I have not built my pull request using AI, a static analysis tool or similar without any human oversight.

Documentation change checklist

My documentation changes follow the docs style guide and any code examples follow the coding style guide.
I tested that my change works before raising the PR (via running doxygen, and testing examples).
I have not moved any existing pages or changed any existing URLs without strong justification as to why.
I have not generated content using AI or a desktop utility such as grammarly.

netlify · 2026-07-07T15:16:33Z

✅ Deploy Preview for dpp-dev ready!

Name	Link
🔨 Latest commit	`d258f53`
🔍 Latest deploy log	https://app.netlify.com/projects/dpp-dev/deploys/6a537e8f680d6c0008cd27ca
😎 Deploy Preview	https://deploy-preview-1608--dpp-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

CLAassistant · 2026-07-07T15:16:42Z

All committers have signed the CLA.

braindigitalis · 2026-07-11T10:13:06Z

there are conflicts in this that have to be resolved.

braindigitalis · 2026-07-11T10:13:42Z

thanks so much for the pr by the way looking forward to merging it!

igor725 · 2026-07-11T10:43:07Z

I'll look into those conflicts in a bit!

igor725 · 2026-07-11T10:44:13Z

thanks so much for the pr by the way looking forward to merging it!

Glad I could help😁

igor725 · 2026-07-11T10:55:22Z

Adjusted it for smart pointers, should be fine now

igor725 · 2026-07-12T12:03:32Z

Just caught that exception again and realised I forgot to move an old object back to shards list in case of said exception. It did the full recreation so technically it worked but now it should be able to resume, too.

igor725 · 2026-07-12T12:09:19Z

Actually now that I think about it more and looking at discordclient.cpp, the old shard can be sabotaged by new constructor since it's moving that connecting_voice_channels map from old object to a new one. We should either come up with mechanism that brings those channels back in case they were already moved and got their ownership swapped or leave it as is and just do full reconnection on exception

braindigitalis · 2026-07-12T14:06:54Z

Actually now that I think about it more and looking at discordclient.cpp, the old shard can be sabotaged by new constructor since it's moving that connecting_voice_channels map from old object to a new one. We should either come up with mechanism that brings those channels back in case they were already moved and got their ownership swapped or leave it as is and just do full reconnection on exception

this is already solved by another pr #1606

igor725 · 2026-07-12T14:44:32Z

Actually now that I think about it more and looking at discordclient.cpp, the old shard can be sabotaged by new constructor since it's moving that connecting_voice_channels map from old object to a new one. We should either come up with mechanism that brings those channels back in case they were already moved and got their ownership swapped or leave it as is and just do full reconnection on exception

this is already solved by another pr #1606

Oh, I missed it. Just checked and I think my changes should be fine as is if git merge won't mess logic up.

Jaskowicz1 · 2026-07-18T20:43:00Z

Corrected the Title and Description to match guidelines

Jaskowicz1 · 2026-07-19T11:36:15Z

I have added @Mishura4 and @Neko-Life to this review.

Neko-Life

Required changes for the missing logic

Neko-Life · 2026-07-19T20:26:16Z

 						log(ll_trace, "Running new connection...");
-						if (resume) {
-							/* Start connecting only after the old client is destroyed */
-							shards[shard_id]->start_connecting();


this resume path shouldn't be removed, resume constructor doesn't invoke start_connecting() anymore

I don't remember removing any resume logic, probably git auto merge shenenigans. I haven't pulled last changes to my branch from the upstream yet so I guess this is the issue. I'm not at my PC right now and next at least two days are pretty busy. I ticked maintainers write access to reconnect-fix branch of my DPP fork, if there's anyone who wants to merge it faster than that, they can push straight to my branch. Otherwise I'll fix it in 2-3 days.

Neko-Life · 2026-07-19T20:29:10Z

 					}
+					log(ll_trace, "Installing new connection...");
+					std::unique_lock lk(shards_mutex);
+					shards[shard_id] = std::move(cl_new);


i think moving the resume path after this line should be correct, including reconnection of the old client's connecting_voice_channels

Fix shard reconnection issue

0986cfd

github-actions Bot added documentation Improvements or additions to documentation code Improvements or additions to code. labels Jul 7, 2026

Merge branch 'dev' into reconnect-fix

b910201

Merge remote-tracking branch 'upstream/dev' into reconnect-fix

2bffc3a

Add missed logic

d258f53

igor725 marked this pull request as draft July 12, 2026 12:09

igor725 marked this pull request as ready for review July 12, 2026 14:44

Jaskowicz1 approved these changes Jul 18, 2026

View reviewed changes

Jaskowicz1 requested review from Mishura4 and Neko-Life July 18, 2026 19:35

Jaskowicz1 changed the title ~~Fix shard reconnection issue (#1607)~~ fix: Fix shard reconnection issue (#1607) Jul 18, 2026

Jaskowicz1 changed the title ~~fix: Fix shard reconnection issue (#1607)~~ fix: Fix shard reconnection issue Jul 18, 2026

Jaskowicz1 changed the title ~~fix: Fix shard reconnection issue~~ fix: Fix shard reconnection issue causing segfault Jul 18, 2026

Jaskowicz1 linked an issue Jul 18, 2026 that may be closed by this pull request

Segfault inside reconnect_monitor #1607

Open

braindigitalis approved these changes Jul 19, 2026

View reviewed changes

Neko-Life requested changes Jul 19, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

igor725 commented Jul 7, 2026 • edited by Jaskowicz1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code change checklist

Documentation change checklist

Uh oh!

netlify Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for dpp-dev ready!

Uh oh!

CLAassistant commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

braindigitalis commented Jul 11, 2026

Uh oh!

braindigitalis commented Jul 11, 2026

Uh oh!

igor725 commented Jul 11, 2026

Uh oh!

igor725 commented Jul 11, 2026

Uh oh!

igor725 commented Jul 11, 2026

Uh oh!

igor725 commented Jul 12, 2026

Uh oh!

igor725 commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

braindigitalis commented Jul 12, 2026

Uh oh!

igor725 commented Jul 12, 2026

Uh oh!

Jaskowicz1 commented Jul 18, 2026

Uh oh!

Jaskowicz1 commented Jul 19, 2026

Uh oh!

Neko-Life left a comment

Choose a reason for hiding this comment

Uh oh!

Neko-Life Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

igor725 Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Neko-Life Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

igor725 commented Jul 7, 2026 •

edited by Jaskowicz1

Loading

netlify Bot commented Jul 7, 2026 •

edited

Loading

CLAassistant commented Jul 7, 2026 •

edited

Loading

igor725 commented Jul 12, 2026 •

edited

Loading