This is an automated email from the ASF dual-hosted git repository.
wwbmmm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new a3280c62 change UserDataDeleter type from function ptr to
std::function (#2431)
a3280c62 is described below
commit a3280c6263ddd83bdb30b598e13a44e4406d064d
Author: youtao guo <[email protected]>
AuthorDate: Tue Jan 23 13:27:07 2024 +0800
change UserDataDeleter type from function ptr to std::function (#2431)
* change UserDataDeleter type from function ptr to std::function
* add unittest
* check that no copy happen when append the stateful user data
* check the destruction is performed correctly
---
src/butil/iobuf.cpp | 15 ++++++++-----
src/butil/iobuf.h | 5 +++--
src/butil/iobuf_inl.h | 4 ++--
test/iobuf_unittest.cpp | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/src/butil/iobuf.cpp b/src/butil/iobuf.cpp
index 169e76ce..b4d6ce3b 100644
--- a/src/butil/iobuf.cpp
+++ b/src/butil/iobuf.cpp
@@ -189,7 +189,7 @@ size_t IOBuf::new_bigview_count() {
}
const uint16_t IOBUF_BLOCK_FLAGS_USER_DATA = 0x1;
-typedef void (*UserDataDeleter)(void*);
+using UserDataDeleter = std::function<void(void*)>;
struct UserDataExtension {
UserDataDeleter deleter;
@@ -233,7 +233,8 @@ struct IOBuf::Block {
, cap(data_size)
, u({0})
, data(data_in) {
- get_user_data_extension()->deleter = deleter;
+ auto ext = new (get_user_data_extension()) UserDataExtension();
+ ext->deleter = std::move(deleter);
}
// Undefined behavior when (flags & IOBUF_BLOCK_FLAGS_USER_DATA) is 0.
@@ -267,7 +268,9 @@ struct IOBuf::Block {
this->~Block();
iobuf::blockmem_deallocate(this);
} else if (flags & IOBUF_BLOCK_FLAGS_USER_DATA) {
- get_user_data_extension()->deleter(data);
+ auto ext = get_user_data_extension();
+ ext->deleter(data);
+ ext->~UserDataExtension();
this->~Block();
free(this);
}
@@ -395,7 +398,7 @@ IOBuf::Block* share_tls_block() {
}
// Return one block to TLS.
-inline void release_tls_block(IOBuf::Block *b) {
+inline void release_tls_block(IOBuf::Block* b) {
if (!b) {
return;
}
@@ -1216,7 +1219,7 @@ int IOBuf::appendv(const const_iovec* vec, size_t n) {
int IOBuf::append_user_data_with_meta(void* data,
size_t size,
- void (*deleter)(void*),
+ std::function<void(void*)> deleter,
uint64_t meta) {
if (size > 0xFFFFFFFFULL - 100) {
LOG(FATAL) << "data_size=" << size << " is too large";
@@ -1233,7 +1236,7 @@ int IOBuf::append_user_data_with_meta(void* data,
if (mem == NULL) {
return -1;
}
- IOBuf::Block* b = new (mem) IOBuf::Block((char*)data, size, deleter);
+ IOBuf::Block* b = new (mem) IOBuf::Block((char*)data, size,
std::move(deleter));
b->u.data_meta = meta;
const IOBuf::BlockRef r = { 0, b->cap, b };
_move_back_ref(r);
diff --git a/src/butil/iobuf.h b/src/butil/iobuf.h
index 5f73f4e0..978f9758 100644
--- a/src/butil/iobuf.h
+++ b/src/butil/iobuf.h
@@ -24,6 +24,7 @@
#include <sys/uio.h> // iovec
#include <stdint.h> // uint32_t
+#include <functional>
#include <string> // std::string
#include <ostream> // std::ostream
#include <google/protobuf/io/zero_copy_stream.h> // ZeroCopyInputStream
@@ -246,11 +247,11 @@ public:
// Append the user-data to back side WITHOUT copying.
// The user-data can be split and shared by smaller IOBufs and will be
// deleted using the deleter func when no IOBuf references it anymore.
- int append_user_data(void* data, size_t size, void (*deleter)(void*));
+ int append_user_data(void* data, size_t size, std::function<void(void*)>
deleter);
// Append the user-data to back side WITHOUT copying.
// The meta is associated with this piece of user-data.
- int append_user_data_with_meta(void* data, size_t size, void
(*deleter)(void*), uint64_t meta);
+ int append_user_data_with_meta(void* data, size_t size,
std::function<void(void*)> deleter, uint64_t meta);
// Get the data meta of the first byte in this IOBuf.
// The meta is specified with append_user_data_with_meta before.
diff --git a/src/butil/iobuf_inl.h b/src/butil/iobuf_inl.h
index f900b82d..c49896a5 100644
--- a/src/butil/iobuf_inl.h
+++ b/src/butil/iobuf_inl.h
@@ -37,8 +37,8 @@ inline ssize_t IOBuf::cut_multiple_into_file_descriptor(
return pcut_multiple_into_file_descriptor(fd, -1, pieces, count);
}
-inline int IOBuf::append_user_data(void* data, size_t size, void
(*deleter)(void*)) {
- return append_user_data_with_meta(data, size, deleter, 0);
+inline int IOBuf::append_user_data(void* data, size_t size,
std::function<void(void*)> deleter) {
+ return append_user_data_with_meta(data, size, std::move(deleter), 0);
}
inline ssize_t IOPortal::append_from_file_descriptor(int fd, size_t max_count)
{
diff --git a/test/iobuf_unittest.cpp b/test/iobuf_unittest.cpp
index e7a6e10d..09dd17da 100644
--- a/test/iobuf_unittest.cpp
+++ b/test/iobuf_unittest.cpp
@@ -20,6 +20,8 @@
#include <sys/socket.h> // socketpair
#include <errno.h> // errno
#include <fcntl.h> // O_RDONLY
+#include <stdlib.h>
+#include <memory>
#include <butil/files/temp_file.h> // TempFile
#include <butil/containers/flat_map.h>
#include <butil/macros.h>
@@ -1616,6 +1618,63 @@ TEST_F(IOBufTest, append_user_data_and_consume) {
}
}
+TEST_F(IOBufTest, append_stateful_user_data) {
+ butil::IOBuf b0;
+ const int REP = 16;
+ const size_t len = REP * 256;
+ std::shared_ptr<char> mem(new char[len], std::default_delete<char[]>());
+ std::weak_ptr<char> weaker = mem;
+ ASSERT_EQ(1, mem.use_count());
+ char* data = mem.get();
+ for (int i = 0; i < 256; ++i) {
+ for (int j = 0; j < REP; ++j) {
+ data[i * REP + j] = (char)i;
+ }
+ }
+
+ int incr_upon_destructrion = 0;
+
+ struct Deleter {
+ Deleter(std::shared_ptr<char> partial, int& incr) :
partial(std::move(partial)), incr(&incr), allow_incr(false) {}
+ ~Deleter() {
+ if (allow_incr) (*incr)++;
+ }
+ Deleter(const Deleter&) { std::abort(); /*if copy, then crash*/ }
+ Deleter(Deleter&&) noexcept = default;
+ void operator()(void*) {
+ partial.reset();
+ allow_incr = true;
+ }
+ std::shared_ptr<char> partial;
+ int* incr;
+ bool allow_incr;
+ };
+
+ for (int i = 0; i < 256; i++) {
+ std::shared_ptr<char> ptr(mem, data + i * REP);
+ ASSERT_EQ(0, b0.append_user_data(data + i * REP, REP,
Deleter{std::move(ptr), incr_upon_destructrion}));
+ }
+ ASSERT_EQ(256, b0._ref_num());
+ ASSERT_EQ(257, mem.use_count());
+ mem.reset();
+ butil::IOBuf::BlockRef r = b0._front_ref();
+ ASSERT_EQ(1, butil::iobuf::block_shared_count(r.block));
+ ASSERT_EQ(len, b0.size());
+ std::string out;
+ ASSERT_EQ(len, b0.cutn(&out, len));
+ ASSERT_TRUE(b0.empty());
+ ASSERT_TRUE(weaker.expired());
+ ASSERT_EQ(256, incr_upon_destructrion);
+
+ ASSERT_EQ(len, out.size());
+ // note: cannot memcmp with data which is already free-ed
+ for (int i = 0; i < 256; ++i) {
+ for (int j = 0; j < REP; ++j) {
+ ASSERT_EQ((char)i, out[i * REP + j]);
+ }
+ }
+}
+
TEST_F(IOBufTest, append_user_data_and_share) {
butil::IOBuf b0;
const int REP = 16;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
