Skip to content

Conversation

zhixinwen
Copy link
Contributor

fix: #3097

warn("Slave thread was terminated, would stop feeding the slave: {}", conn_->GetAddr());
}

void FeedSlaveThread::Join() {
if (auto s = util::ThreadJoin(t_); !s) {
warn("Slave thread operation failed: {}", s.Msg());
} else {
// after the loop is finished, the connection can be freed
conn_.reset();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not find where the connection is released after FeedSlaveThread stops. Please let me know if this should not be done.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it should be correct to reset the unique pointer only here. Because the connection has been detached from the worker before using it in FeedSlaveThread, so it's nothing to be done.

@@ -622,3 +625,126 @@ func TestSlaveLostMaster(t *testing.T) {
duration := time.Since(start)
require.Less(t, duration, time.Second*6)
}

func TestWALDiscreteError(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to create a WAL with discrete sequence number to trigger the error, but I cannot trigger the seg fault. I think it is because the seg fault only happens when ack is sent back on the same connection after FeedSlaveThread::Stop call and the time window is small.

I am leaving it here because it does no harm to test an extra case, but let me know if there is better way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, this is not related to the seg fault

@zhixinwen zhixinwen force-pushed the zhixin/fix-seg-fault branch 5 times, most recently from 6c89761 to 2da98d2 Compare August 5, 2025 22:20
@zhixinwen zhixinwen force-pushed the zhixin/fix-seg-fault branch from 2da98d2 to 95ecf41 Compare August 5, 2025 22:21
@zhixinwen zhixinwen closed this Aug 7, 2025
@zhixinwen zhixinwen reopened this Aug 7, 2025
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.

segfault in FeedSlaveThread
2 participants