Skip to content

Commit

Permalink
Allow deserialize during cursor read
Browse files Browse the repository at this point in the history
Summary: There's nothing wrong with concurrent reads where only one needs access to cursor state.

Reviewed By: Mizuchi

Differential Revision: D66831490

fbshipit-source-id: c6d38adb0a8ab6cc74b43ff4ea719389d0763347
  • Loading branch information
iahs authored and facebook-github-bot committed Dec 5, 2024
1 parent ea727a1 commit d198c85
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
#pragma once

#include <algorithm>
#include <initializer_list>
#include <variant>
#include <folly/io/Cursor.h>
#include <folly/io/IOBufQueue.h>
#include <thrift/lib/cpp2/protocol/BinaryProtocol.h>
#include <thrift/lib/cpp2/protocol/Serializer.h>
#include <thrift/lib/cpp2/protocol/detail/CursorBasedSerialization.h>
#include <thrift/lib/cpp2/type/NativeType.h>
#include <thrift/lib/cpp2/type/ThriftType.h>
Expand Down Expand Up @@ -70,6 +70,7 @@ class CursorSerializationWrapper {
static_assert(
std::is_same_v<ProtocolWriter, BinaryProtocolWriter>,
"ProtocolWriter must be BinaryProtocolReader");
using Serializer = Serializer<ProtocolReader, ProtocolWriter>;

public:
CursorSerializationWrapper() = default;
Expand Down Expand Up @@ -100,12 +101,13 @@ class CursorSerializationWrapper {
* Object read path (traditional Thrift deserialization)
* Deserializes into a (returned) Thrift object.
*/
T deserialize() {
T deserialize() const {
if (std::holds_alternative<ProtocolWriter>(protocol_)) {
folly::throw_exception<std::runtime_error>(
"Concurrent reads/writes not supported");
}
checkHasData();
T ret;
ret.read(reader());
done();
return ret;
return Serializer::template deserialize<T>(serializedData_.get());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#include <thrift/lib/cpp2/protocol/CursorBasedSerializer.h>

#include <ranges>
#include <folly/portability/GMock.h>
#include <folly/portability/GTest.h>
#include <thrift/lib/cpp2/protocol/test/gen-cpp2/cursor_clients.h>
Expand Down Expand Up @@ -686,3 +685,18 @@ TEST(CursorBasedSerializer, CursorReadRemainingEndOne) {
TEST(CursorBasedSerializer, CursorReadRemainingEndMany) {
doCursorReadRemainEndTest(10);
}

TEST(CursorBasedSerializer, ConcurrentAccess) {
EmptyWrapper wrapper;
auto writer = wrapper.beginWrite();
EXPECT_THROW(wrapper.beginRead(), std::runtime_error);
EXPECT_THROW(wrapper.deserialize(), std::runtime_error);
EXPECT_THROW(wrapper.beginWrite(), std::runtime_error);
wrapper.endWrite(std::move(writer));

auto reader = wrapper.beginRead();
EXPECT_THROW(wrapper.beginRead(), std::runtime_error);
EXPECT_EQ(wrapper.deserialize(), Empty{});
EXPECT_THROW(wrapper.beginWrite(), std::runtime_error);
wrapper.endRead(std::move(reader));
}

0 comments on commit d198c85

Please sign in to comment.