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 #5134 segmentfault #5138

Merged
merged 4 commits into from
Aug 30, 2023
Merged

Fix #5134 segmentfault #5138

merged 4 commits into from
Aug 30, 2023

Conversation

NathanFreeman
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Merging #5138 (ae9d416) into master (664e3fd) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5138      +/-   ##
==========================================
+ Coverage   72.93%   72.99%   +0.06%     
==========================================
  Files          69       69              
  Lines       14848    14852       +4     
==========================================
+ Hits        10829    10841      +12     
+ Misses       4019     4011       -8     
Files Changed Coverage Δ
src/server/master.cc 74.55% <100.00%> (+0.08%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -1318,6 +1318,7 @@ class Server {
}

int create_pipe_buffers();
void free_pipe_buffers();
Copy link
Contributor

Choose a reason for hiding this comment

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

下面已经有个release_pipe_buffers()的定义,没有必要再定义free_pipe_buffers了吧

Copy link
Member Author

Choose a reason for hiding this comment

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

原来还有release_pipe_buffers这个定义,但是这个没有实现,待会把free_pipe_buffers直接改名成release_pipe_buffers好了

@@ -187,6 +187,7 @@ void Manager::wait(Server *_server) {
delete[] pool->reload_workers;
pool->reload_workers = nullptr;
server_->manager = nullptr;
server_->free_pipe_buffers();
Copy link
Contributor

Choose a reason for hiding this comment

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

free_pipe_buffers();放在Server::destroy()里面比较好,SWOOLE_BASE模式下,worker_num=1这里不会得到执行

@@ -753,6 +753,7 @@ Server::~Server() {
delete port;
}
sw_shm_free(gs);
free_pipe_buffers();
Copy link
Contributor

Choose a reason for hiding this comment

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

如果free_pipe_buffers();放在Server::destroy()里面,这里就没有必要了

@matyhtf matyhtf merged commit d0b7ec0 into swoole:master Aug 30, 2023
26 checks passed
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