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

Refactor: Replace libevent with libuvw in Craned #359

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

L-Xiafeng
Copy link
Collaborator

No description provided.

@L-Xiafeng L-Xiafeng added refactor Code refactor build Build system change labels Nov 12, 2024
@L-Xiafeng L-Xiafeng force-pushed the refactor/uvw branch 2 times, most recently from dd082ef to 96f7e03 Compare November 12, 2024 09:04
@L-Xiafeng L-Xiafeng marked this pull request as ready for review November 12, 2024 09:05
@L-Xiafeng L-Xiafeng changed the title Refactor: replace libeven by libuvw in craned Refactor: Replace libeven by libuvw in craned Nov 12, 2024
@Nativu5 Nativu5 changed the title Refactor: Replace libeven by libuvw in craned Refactor: Replace libevent with libuvw in Craned Nov 12, 2024
@@ -875,8 +770,8 @@ CraneErr TaskManager::SpawnProcessInInstance_(TaskInstance* instance,
stderr_fd =
open(stderr_file_path.c_str(), O_RDWR | O_CREAT | O_TRUNC, 0644);
if (stderr_fd == -1) {
CRANE_ERROR("[Child Process] Error: open {}. {}", stderr_file_path,
strerror(errno));
// CRANE_ERROR("[Child Process] Error: open {}. {}", stderr_file_path,
Copy link
Collaborator

Choose a reason for hiding this comment

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

这注释留着也没啥吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

会死锁,因为spdlog内部也有互斥锁

int err = istream.GetErrno();
CRANE_ERROR("Failed to read socket from parent: {}", strerror(err));
}
// if (!ok) {
Copy link
Collaborator

@RileyWen RileyWen Nov 12, 2024

Choose a reason for hiding this comment

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

这LOG为啥关了

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

spdlog内部也有互斥锁,导致子进程死锁,所以里面的log全关了,而且本来也打不出来日志。


if (!this_->m_is_ending_now_) {
void TaskManager::SigintCb_() {
absl::MutexLock lock_guard(&m_mtx_);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这边为啥多了一个锁

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

asan检测出来的,不加锁可能会和LaunchTaskInstanceMt_有datarace

@L-Xiafeng L-Xiafeng force-pushed the refactor/uvw branch 2 times, most recently from f5f02ea to c5f0663 Compare November 14, 2024 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system change refactor Code refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants