You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by ja...@apache.org on 2020/07/24 08:05:05 UTC
[incubator-brpc] branch master updated:
Controller::CreateProgressiveAttachment returns intrusive_ptr instead of
raw pointer which could be deleted and invalid before usage
This is an automated email from the ASF dual-hosted git repository.
jamesge pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 2071a22 Controller::CreateProgressiveAttachment returns intrusive_ptr instead of raw pointer which could be deleted and invalid before usage
2071a22 is described below
commit 2071a22c7f8ca4dfd47b1f0b607737082b66c865
Author: jamesge <jg...@gmail.com>
AuthorDate: Fri Jul 24 16:04:24 2020 +0800
Controller::CreateProgressiveAttachment returns intrusive_ptr instead of raw pointer which could be deleted and invalid before usage
---
docs/cn/http_service.md | 2 +-
docs/en/http_service.md | 2 +-
example/http_c++/http_client.cpp | 2 +-
example/http_c++/http_server.cpp | 18 +++++++++++-------
src/brpc/controller.cpp | 9 ++++-----
src/brpc/controller.h | 3 ++-
test/brpc_http_rpc_protocol_unittest.cpp | 8 ++++----
7 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/docs/cn/http_service.md b/docs/cn/http_service.md
index c8b6827..6aaa4b8 100644
--- a/docs/cn/http_service.md
+++ b/docs/cn/http_service.md
@@ -337,7 +337,7 @@ brpc server支持发送超大或无限长的body。方法如下:
```c++
#include <brpc/progressive_attachment.h>
...
- butil::intrusive_ptr<brpc::ProgressiveAttachment> pa(cntl->CreateProgressiveAttachment());
+ butil::intrusive_ptr<brpc::ProgressiveAttachment> pa = cntl->CreateProgressiveAttachment();
```
2. 调用ProgressiveAttachment::Write()发送数据。
diff --git a/docs/en/http_service.md b/docs/en/http_service.md
index c0749ad..fc403bc 100644
--- a/docs/en/http_service.md
+++ b/docs/en/http_service.md
@@ -338,7 +338,7 @@ brpc server is capable of sending large or infinite sized body, in following ste
```c++
#include <brpc/progressive_attachment.h>
...
- butil::intrusive_ptr<brpc::ProgressiveAttachment> pa (cntl->CreateProgressiveAttachment());
+ butil::intrusive_ptr<brpc::ProgressiveAttachment> pa = cntl->CreateProgressiveAttachment();
```
2. Call `ProgressiveAttachment::Write()` to send the data.
diff --git a/example/http_c++/http_client.cpp b/example/http_c++/http_client.cpp
index 6d5e0e9..d9984c8 100644
--- a/example/http_c++/http_client.cpp
+++ b/example/http_c++/http_client.cpp
@@ -28,7 +28,7 @@
DEFINE_string(d, "", "POST this data to the http server");
DEFINE_string(load_balancer, "", "The algorithm for load balancing");
-DEFINE_int32(timeout_ms, 1000, "RPC timeout in milliseconds");
+DEFINE_int32(timeout_ms, 2000, "RPC timeout in milliseconds");
DEFINE_int32(max_retry, 3, "Max retries(not including the first RPC)");
DEFINE_string(protocol, "http", "Client-side protocol");
diff --git a/example/http_c++/http_server.cpp b/example/http_c++/http_server.cpp
index c52800e..d20ee71 100644
--- a/example/http_c++/http_server.cpp
+++ b/example/http_c++/http_server.cpp
@@ -69,17 +69,20 @@ public:
FileServiceImpl() {};
virtual ~FileServiceImpl() {};
- static void* SendLargeFile(void* arg) {
- butil::intrusive_ptr<brpc::ProgressiveAttachment> pa(
- (brpc::ProgressiveAttachment*)arg);
- if (pa == NULL) {
+ struct Args {
+ butil::intrusive_ptr<brpc::ProgressiveAttachment> pa;
+ };
+
+ static void* SendLargeFile(void* raw_args) {
+ std::unique_ptr<Args> args(static_cast<Args*>(raw_args));
+ if (args->pa == NULL) {
LOG(ERROR) << "ProgressiveAttachment is NULL";
return NULL;
}
for (int i = 0; i < 100; ++i) {
char buf[16];
int len = snprintf(buf, sizeof(buf), "part_%d ", i);
- pa->Write(buf, len);
+ args->pa->Write(buf, len);
// sleep a while to send another part.
bthread_usleep(10000);
@@ -97,9 +100,10 @@ public:
const std::string& filename = cntl->http_request().unresolved_path();
if (filename == "largefile") {
// Send the "largefile" with ProgressiveAttachment.
+ std::unique_ptr<Args> args(new Args);
+ args->pa = cntl->CreateProgressiveAttachment();
bthread_t th;
- bthread_start_background(&th, NULL, SendLargeFile,
- cntl->CreateProgressiveAttachment());
+ bthread_start_background(&th, NULL, SendLargeFile, args.release());
} else {
cntl->response_attachment().append("Getting file: ");
cntl->response_attachment().append(filename);
diff --git a/src/brpc/controller.cpp b/src/brpc/controller.cpp
index 8455508..ace380f 100644
--- a/src/brpc/controller.cpp
+++ b/src/brpc/controller.cpp
@@ -1345,7 +1345,7 @@ void Controller::set_stream_creator(StreamCreator* sc) {
_stream_creator = sc;
}
-ProgressiveAttachment*
+butil::intrusive_ptr<ProgressiveAttachment>
Controller::CreateProgressiveAttachment(StopStyle stop_style) {
if (has_progressive_writer()) {
LOG(ERROR) << "One controller can only have one ProgressiveAttachment";
@@ -1365,10 +1365,9 @@ Controller::CreateProgressiveAttachment(StopStyle stop_style) {
if (stop_style == FORCE_STOP) {
httpsock->fail_me_at_server_stop();
}
- ProgressiveAttachment* pb = new ProgressiveAttachment(
- httpsock, http_request().before_http_1_1());
- _wpa.reset(pb);
- return pb;
+ _wpa.reset(new ProgressiveAttachment(
+ httpsock, http_request().before_http_1_1()));
+ return _wpa;
}
void Controller::ReadProgressiveAttachmentBy(ProgressiveReader* r) {
diff --git a/src/brpc/controller.h b/src/brpc/controller.h
index 69b3f48..19a352e 100755
--- a/src/brpc/controller.h
+++ b/src/brpc/controller.h
@@ -380,8 +380,9 @@ public:
// If `stop_style' is FORCE_STOP, the underlying socket will be failed
// immediately when the socket becomes idle or server is stopped.
// Default value of `stop_style' is WAIT_FOR_STOP.
- ProgressiveAttachment*
+ butil::intrusive_ptr<ProgressiveAttachment>
CreateProgressiveAttachment(StopStyle stop_style = WAIT_FOR_STOP);
+
bool has_progressive_writer() const { return _wpa != NULL; }
// Set compression method for response.
diff --git a/test/brpc_http_rpc_protocol_unittest.cpp b/test/brpc_http_rpc_protocol_unittest.cpp
index 0eb4760..15ef84b 100644
--- a/test/brpc_http_rpc_protocol_unittest.cpp
+++ b/test/brpc_http_rpc_protocol_unittest.cpp
@@ -499,8 +499,8 @@ public:
cntl->http_response().set_content_type("text/plain");
brpc::StopStyle stop_style = (_nrep == std::numeric_limits<size_t>::max()
? brpc::FORCE_STOP : brpc::WAIT_FOR_STOP);
- butil::intrusive_ptr<brpc::ProgressiveAttachment> pa(
- cntl->CreateProgressiveAttachment(stop_style));
+ butil::intrusive_ptr<brpc::ProgressiveAttachment> pa
+ = cntl->CreateProgressiveAttachment(stop_style);
if (pa == NULL) {
cntl->SetFailed("The socket was just failed");
return;
@@ -547,8 +547,8 @@ public:
cntl->http_response().set_content_type("text/plain");
brpc::StopStyle stop_style = (_nrep == std::numeric_limits<size_t>::max()
? brpc::FORCE_STOP : brpc::WAIT_FOR_STOP);
- butil::intrusive_ptr<brpc::ProgressiveAttachment> pa(
- cntl->CreateProgressiveAttachment(stop_style));
+ butil::intrusive_ptr<brpc::ProgressiveAttachment> pa
+ = cntl->CreateProgressiveAttachment(stop_style);
if (pa == NULL) {
cntl->SetFailed("The socket was just failed");
return;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org