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

CheckConnectedAndKeepWrite check失败 #2490

Closed
pavel2003 opened this issue Jan 2, 2024 · 2 comments · Fixed by #2776
Closed

CheckConnectedAndKeepWrite check失败 #2490

pavel2003 opened this issue Jan 2, 2024 · 2 comments · Fixed by #2776

Comments

@pavel2003
Copy link

Describe the bug (描述bug)

如果Close的时候还没有设置_connected的话(stream设置的时候socket被对端关掉就会是这种场景),那么CheckConnectedAndKeepWrite里面的CHECK_GE会一直失败,打印调用栈

那这个check的意义在哪呢

同时问一下 // Trigger on connect to release the reference of socket
这里的reference是对应哪里的呢,感觉stream的connect和socket不一样,没有req对应的reference吧?

void Stream::Close() {
_fake_socket_weak_ref->SetFailed();
bthread_mutex_lock(&_connect_mutex);
if (_closed) {
bthread_mutex_unlock(&_connect_mutex);
return;
}
_closed = true;
if (_connected) {
bthread_mutex_unlock(&_connect_mutex);
return;
}
_connect_meta.ec = ECONNRESET;
// Trigger on connect to release the reference of socket
return TriggerOnConnectIfNeed();
}

void* Stream::RunOnConnect(void arg) {
ConnectMeta
meta = (ConnectMeta*)arg;
if (meta->ec == 0) {
meta->on_connect(Socket::STREAM_FAKE_FD, 0, meta->arg);
} else {
meta->on_connect(-1, meta->ec, meta->arg);
}
delete meta;
return NULL;
}

void Socket::CheckConnectedAndKeepWrite(int fd, int err, void* data) {
butil::fd_guard sockfd(fd);
WriteRequest* req = static_cast<WriteRequest*>(data);
Socket* s = req->socket;
CHECK_GE(sockfd, 0);

To Reproduce (复现方法)

Expected behavior (期望行为)

Versions (各种版本)
OS:
Compiler:
brpc:
protobuf:

Additional context/screenshots (更多上下文/截图)

@chongg039
Copy link

也遇到了这个问题,请问后来是如何解决的

@chenBright
Copy link
Contributor

应该是stream的bug,简单修复一下的话,可以在有conn的时候不check。

同时问一下 // Trigger on connect to release the reference of socket
这里的reference是对应哪里的呢,感觉stream的connect和socket不一样,没有req对应的reference吧?

对应的是建连接的时候持有了一个引用的。

brpc/src/brpc/socket.cpp

Lines 1414 to 1420 in f17048e

// Have to hold a reference for `req'
SocketUniquePtr s;
ReAddress(&s);
req->set_socket(s.get());
if (DoConnect(abstime, KeepWriteIfConnected, req) < 0) {
return -1;
}

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 a pull request may close this issue.

3 participants