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

fix: fix pika coredump when process exit #1820

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

wangshao1
Copy link
Collaborator

修复pika进程退出时的coredump。 #1819

出现问题的原因是pika_repl_server在执行FdClosedHandle时会用到pika_server,pika_server已经析构了,所以coredump.
修复方式是pika_repl_server在stop时,关闭掉连接。

PikaReplServer cleanup connections when destroy function called, cleanup will trigger FdClosedHandle which calls already destroyed pikaserver.
So manually call cleanup ahead before pikaserver destroyed.
@@ -72,7 +73,6 @@ class HolyThread : public ServerThread {
void HandleConnEvent(NetFiredEvent* pfe) override;

void CloseFd(const std::shared_ptr<NetConn>& conn);
void Cleanup();
}; // class HolyThread

} // namespace net
Copy link

Choose a reason for hiding this comment

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

Based on the provided code patch, here's a brief code review:

  1. In the HolyThread class, you have added a new method called Cleanup(). Make sure to implement this method correctly.

  2. You have marked Cleanup() as both override and private. If this method is meant to override a base class method, ensure that the signature and return type match exactly.

  3. In the HolyThread class, the method HandleConnEvent(NetFiredEvent* pfe) seems to be missing from the code patch. Check if it has been mistakenly omitted or moved elsewhere.

  4. The method CloseFd(const std::shared_ptr<NetConn>& conn) should be implemented according to its purpose to ensure proper cleanup and closure of file descriptors.

Overall, without having the complete codebase, it's difficult to identify additional bug risks or suggest specific improvements. It is important to thoroughly test the modified code and handle error conditions appropriately. Additionally, ensure that the changes align with the intended functionality and adhere to the best coding practices of the existing codebase.

@AlexStocks AlexStocks merged commit 978725e into OpenAtomFoundation:unstable Jul 26, 2023
9 checks passed
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
PikaReplServer cleanup connections when destroy function called, cleanup will trigger FdClosedHandle which calls already destroyed pikaserver.
So manually call cleanup ahead before pikaserver destroyed.

Co-authored-by: wangshaoyi <[email protected]>
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
PikaReplServer cleanup connections when destroy function called, cleanup will trigger FdClosedHandle which calls already destroyed pikaserver.
So manually call cleanup ahead before pikaserver destroyed.

Co-authored-by: wangshaoyi <[email protected]>
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.

3 participants