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: incorrect manner of terminating the rsync process #1596

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

machinly
Copy link
Collaborator

@machinly machinly commented Jun 7, 2023

现有代码中 kill rsync 的方法 kill $(ps -o pgid=" + std::to_string(pid) + ")" 有问题。

ps -o pgid=12345 的含义是列出的活动进程组ID,并把 header "PGID" 改为 12345。

kill 掉这个命令返回结果会 kill 所有当前所有的活动进程。

这个 PR 将其修改为 kill -- -$(ps -o pgid -p " + std::to_string(pid) + " | grep -o '[0-9]*')"

意图为:

列出 pid 为 12345 的进程的进程组 id ps -o pgid -p 12345,并过滤掉非数字结果,得到所需的进程组 id 666。

然后 kill 掉进程组 id 为 666 的所有进程 kill -- -666

@@ -114,7 +114,7 @@ int StopRsync(const std::string& raw_path) {
return 0;
}

std::string rsync_stop_cmd = "kill $(ps -o pgid=" + std::to_string(pid) + ")";
std::string rsync_stop_cmd = "kill -- -$(ps -o pgid -p" + std::to_string(pid) + " | grep -o '[0-9]*')";
int ret = system(rsync_stop_cmd.c_str());
if (ret == 0 || (WIFEXITED(ret) && !WEXITSTATUS(ret))) {
LOG(INFO) << "Stop rsync success!";
Copy link

Choose a reason for hiding this comment

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

The reviewed code is a patch for stopping an rsync process. The changes made in the patch include modifying the command used to stop the process by using the pgid (process group id) instead of the pid (process ID). This allows all child processes spawned by the rsync process to be stopped as well.

Improvement suggestions:

  • It's recommended to add error handling for the system() call, so any failure can be properly reported.
  • Instead of using system(), it's better to use fork()/exec() and waitpid() functions for better control over the execution of the command and avoiding potential security issues.
  • It's also good to perform null checks on the returned pointer value of std::to_string(pid) to avoid runtime errors caused by invalid or uninitialized inputs.

@AlexStocks
Copy link
Collaborator

我在mac上测试确实没啥问题,在 centos 上测试也没问题

@AlexStocks AlexStocks merged commit 27c858c into OpenAtomFoundation:unstable Jun 7, 2023
@machinly machinly deleted the fix_kill_rsync branch June 12, 2023 03:15
lqxhub pushed a commit to lqxhub/pika that referenced this pull request Jun 18, 2023
AlexStocks added a commit that referenced this pull request Jul 2, 2023
* feat: supported 'ptype' command

* Simplified command `type` and `ptype` code

* Remove the capacity judgment

* fix slavaof serialize response bug (#1583)

Co-authored-by: liuyuecai <liuyuecai@360.cn>

* fix (#1587) (#1588)

* fix unit test:type/set (#1577)

* Fix memory leaks in HandleBGWorkerWriteDB and HandleMetaSyncResponse (#1590)

* fix memory leaks in PikaReplBgWorker::HandleBGWorkerWriteDB(void* arg) and PikaReplClientConn::HandleMetaSyncResponse(void* arg).

* add address/thread sanitizer to CMakeLists

---------

Co-authored-by: cjh <1271435567@qq.com>

* refactor: replace pstd/env with std::filesystem (#1470)

* fix bug issue 1554: using unique_ptr.reset to fix the SIGABRT error (#1595)

using unique_ptr.reset to fix the SIGABRT error && using time-wait lock to notify quickly

* fix: incorrect manner of terminating the process (#1596)

* fix issue#1597: add rocksdb dependency to pstd (#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix_info_command

* fix: gcc13 compile failed (#1601)

* ci: add unit test to github action (#1604)

* ci: add unit test in github workflow

* chore:change `PLATFORM` field logic (#1615)

* fix issue 1517: scan 命令不支持 type 参数 (#1582)

* fix: the unit test of type/set (#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* [fix issue1621] fix deadlock (#1620)

* [fix] fix deadlock

* [fix] fix

* command `type` and `ptype` add unit test

* fix member variable initialization

* Update issue templates

* feat: supported 'ptype' command

* fix unit test:type/set (#1577)

* fix issue#1597: add rocksdb dependency to pstd (#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix: gcc13 compile failed (#1601)

* fix: the unit test of type/set (#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* Modify other modules to use the new `type` function

* fix function repeat

---------

Co-authored-by: Yuecai Liu <38887641+luky116@users.noreply.github.com>
Co-authored-by: liuyuecai <liuyuecai@360.cn>
Co-authored-by: Peter Chan <luckygoose@foxmail.com>
Co-authored-by: chenbt <34958405+chenbt-hz@users.noreply.github.com>
Co-authored-by: Junhua Chen <41671101+cheniujh@users.noreply.github.com>
Co-authored-by: cjh <1271435567@qq.com>
Co-authored-by: kang jinci <jincikang@gmail.com>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: machinly <machinlyg@gmail.com>
Co-authored-by: A2ureStone <73770413+A2ureStone@users.noreply.github.com>
Co-authored-by: J1senn <J1senn@outlook.com>
Co-authored-by: chejinge <945997690@qq.com>
Co-authored-by: baerwang <52104949+baerwang@users.noreply.github.com>
Co-authored-by: ptbxzrt <89020404+ptbxzrt@users.noreply.github.com>
Co-authored-by: 你不要过来啊 <73388438+iiiuwioajdks@users.noreply.github.com>
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
* feat: supported 'ptype' command

* Simplified command `type` and `ptype` code

* Remove the capacity judgment

* fix slavaof serialize response bug (OpenAtomFoundation#1583)

Co-authored-by: liuyuecai <liuyuecai@360.cn>

* fix (OpenAtomFoundation#1587) (OpenAtomFoundation#1588)

* fix unit test:type/set (OpenAtomFoundation#1577)

* Fix memory leaks in HandleBGWorkerWriteDB and HandleMetaSyncResponse (OpenAtomFoundation#1590)

* fix memory leaks in PikaReplBgWorker::HandleBGWorkerWriteDB(void* arg) and PikaReplClientConn::HandleMetaSyncResponse(void* arg).

* add address/thread sanitizer to CMakeLists

---------

Co-authored-by: cjh <1271435567@qq.com>

* refactor: replace pstd/env with std::filesystem (OpenAtomFoundation#1470)

* fix bug issue 1554: using unique_ptr.reset to fix the SIGABRT error (OpenAtomFoundation#1595)

using unique_ptr.reset to fix the SIGABRT error && using time-wait lock to notify quickly

* fix: incorrect manner of terminating the process (OpenAtomFoundation#1596)

* fix issue#1597: add rocksdb dependency to pstd (OpenAtomFoundation#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix_info_command

* fix: gcc13 compile failed (OpenAtomFoundation#1601)

* ci: add unit test to github action (OpenAtomFoundation#1604)

* ci: add unit test in github workflow

* chore:change `PLATFORM` field logic (OpenAtomFoundation#1615)

* fix issue 1517: scan 命令不支持 type 参数 (OpenAtomFoundation#1582)

* fix: the unit test of type/set (OpenAtomFoundation#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* [fix issue1621] fix deadlock (OpenAtomFoundation#1620)

* [fix] fix deadlock

* [fix] fix

* command `type` and `ptype` add unit test

* fix member variable initialization

* Update issue templates

* feat: supported 'ptype' command

* fix unit test:type/set (OpenAtomFoundation#1577)

* fix issue#1597: add rocksdb dependency to pstd (OpenAtomFoundation#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix: gcc13 compile failed (OpenAtomFoundation#1601)

* fix: the unit test of type/set (OpenAtomFoundation#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* Modify other modules to use the new `type` function

* fix function repeat

---------

Co-authored-by: Yuecai Liu <38887641+luky116@users.noreply.github.com>
Co-authored-by: liuyuecai <liuyuecai@360.cn>
Co-authored-by: Peter Chan <luckygoose@foxmail.com>
Co-authored-by: chenbt <34958405+chenbt-hz@users.noreply.github.com>
Co-authored-by: Junhua Chen <41671101+cheniujh@users.noreply.github.com>
Co-authored-by: cjh <1271435567@qq.com>
Co-authored-by: kang jinci <jincikang@gmail.com>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: machinly <machinlyg@gmail.com>
Co-authored-by: A2ureStone <73770413+A2ureStone@users.noreply.github.com>
Co-authored-by: J1senn <J1senn@outlook.com>
Co-authored-by: chejinge <945997690@qq.com>
Co-authored-by: baerwang <52104949+baerwang@users.noreply.github.com>
Co-authored-by: ptbxzrt <89020404+ptbxzrt@users.noreply.github.com>
Co-authored-by: 你不要过来啊 <73388438+iiiuwioajdks@users.noreply.github.com>
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
cheniujh added a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
* feat: supported 'ptype' command

* Simplified command `type` and `ptype` code

* Remove the capacity judgment

* fix slavaof serialize response bug (OpenAtomFoundation#1583)

Co-authored-by: liuyuecai <liuyuecai@360.cn>

* fix (OpenAtomFoundation#1587) (OpenAtomFoundation#1588)

* fix unit test:type/set (OpenAtomFoundation#1577)

* Fix memory leaks in HandleBGWorkerWriteDB and HandleMetaSyncResponse (OpenAtomFoundation#1590)

* fix memory leaks in PikaReplBgWorker::HandleBGWorkerWriteDB(void* arg) and PikaReplClientConn::HandleMetaSyncResponse(void* arg).

* add address/thread sanitizer to CMakeLists

---------

Co-authored-by: cjh <1271435567@qq.com>

* refactor: replace pstd/env with std::filesystem (OpenAtomFoundation#1470)

* fix bug issue 1554: using unique_ptr.reset to fix the SIGABRT error (OpenAtomFoundation#1595)

using unique_ptr.reset to fix the SIGABRT error && using time-wait lock to notify quickly

* fix: incorrect manner of terminating the process (OpenAtomFoundation#1596)

* fix issue#1597: add rocksdb dependency to pstd (OpenAtomFoundation#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix_info_command

* fix: gcc13 compile failed (OpenAtomFoundation#1601)

* ci: add unit test to github action (OpenAtomFoundation#1604)

* ci: add unit test in github workflow

* chore:change `PLATFORM` field logic (OpenAtomFoundation#1615)

* fix issue 1517: scan 命令不支持 type 参数 (OpenAtomFoundation#1582)

* fix: the unit test of type/set (OpenAtomFoundation#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* [fix issue1621] fix deadlock (OpenAtomFoundation#1620)

* [fix] fix deadlock

* [fix] fix

* command `type` and `ptype` add unit test

* fix member variable initialization

* Update issue templates

* feat: supported 'ptype' command

* fix unit test:type/set (OpenAtomFoundation#1577)

* fix issue#1597: add rocksdb dependency to pstd (OpenAtomFoundation#1599)

* fix g++15 compile failure

* add rocksdb dependency to pstd

---------

Co-authored-by: J1senn <J1senn@outlook.com>

* fix: gcc13 compile failed (OpenAtomFoundation#1601)

* fix: the unit test of type/set (OpenAtomFoundation#1617)

* test: optimize the return results of srandmember to avoid approximate results

* fix: use last_seed for random engine

* Modify other modules to use the new `type` function

* fix function repeat

---------

Co-authored-by: Yuecai Liu <38887641+luky116@users.noreply.github.com>
Co-authored-by: liuyuecai <liuyuecai@360.cn>
Co-authored-by: Peter Chan <luckygoose@foxmail.com>
Co-authored-by: chenbt <34958405+chenbt-hz@users.noreply.github.com>
Co-authored-by: Junhua Chen <41671101+cheniujh@users.noreply.github.com>
Co-authored-by: cjh <1271435567@qq.com>
Co-authored-by: kang jinci <jincikang@gmail.com>
Co-authored-by: Xin.Zh <dragoncharlie@foxmail.com>
Co-authored-by: machinly <machinlyg@gmail.com>
Co-authored-by: A2ureStone <73770413+A2ureStone@users.noreply.github.com>
Co-authored-by: J1senn <J1senn@outlook.com>
Co-authored-by: chejinge <945997690@qq.com>
Co-authored-by: baerwang <52104949+baerwang@users.noreply.github.com>
Co-authored-by: ptbxzrt <89020404+ptbxzrt@users.noreply.github.com>
Co-authored-by: 你不要过来啊 <73388438+iiiuwioajdks@users.noreply.github.com>
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