Skip to content

Commit 2e6a99f

Browse files
hs0225hs0225
committed
fix(message-port): fix double free error
Signed-off-by: Hosung Kim hs852.kim@samsung.com
1 parent db576cd commit 2e6a99f

6 files changed

Lines changed: 75 additions & 57 deletions

File tree

‎deps/message-port/expected.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ class Expected {
5656
Expected(T&& v) : value_(std::move(v)), has_value_(true) {}
5757
Expected(const E& e) : error_(e), has_value_(false) {}
5858
Expected(E&& e) : error_(std::move(e)), has_value_(false) {}
59-
~Expected() { has_value_ ? value_.~T() : error_.~E(); }
59+
~Expected() {
60+
if (!has_value_) error_.~E();
61+
}
6062

6163
bool valid() const { return has_value_; }
6264
T& value() { return value_; }

‎deps/message-port/message-port.cc‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,37 +75,27 @@ const std::weak_ptr<Port>& MessageEvent::target() const {
7575
return internal_->target;
7676
}
7777

78-
// MessageEventSync::Internal
78+
// MessageEventSync
7979
// -----------------------------------------------------------------------------
80-
struct MessageEventSync::Internal {
81-
Internal() {}
82-
~Internal() { TRACE(MSGEVENT, "~MessageEventSync::Internal"); }
8380

84-
std::promise<std::string> promise;
85-
};
81+
MessageEventSync::HandleData::HandleData() {}
82+
83+
MessageEventSync::HandleData::~HandleData() {
84+
TRACE(MSGEVENT, "MessageEventSync::HandleData::~HandleData");
85+
}
8686

8787
std::shared_ptr<MessageEventSync> MessageEventSync::New(
8888
const std::string& data) {
8989
return std::shared_ptr<MessageEventSync>(new MessageEventSync(data));
9090
}
9191

9292
MessageEventSync::MessageEventSync(const std::string& data)
93-
: MessageEvent(data), internal_sync_(std::make_unique<Internal>()) {}
93+
: MessageEvent(data), handle_data_(std::make_shared<HandleData>()) {}
9494

9595
MessageEventSync::~MessageEventSync() {
9696
TRACE(MSGEVENT, "~MessageEventSync");
9797
}
9898

99-
void MessageEventSync::SetResult(const std::string& result) const {
100-
try {
101-
// If the user sets the result before in onmessage callback, set the
102-
// promise value. Otherwise, The promise will be unresolved forever.
103-
internal_sync_->promise.set_value(result);
104-
} catch (const std::exception& e) {
105-
LWNODE_DEV_LOG("[MessageEventSync::SetResult] promise error:", e.what());
106-
return;
107-
}
108-
}
10999

110100
// Port::Internal
111101
// -----------------------------------------------------------------------------
@@ -224,7 +214,7 @@ Port::Result Port::PostMessage(std::shared_ptr<MessageEventSync> event,
224214
// cause a deadlock. It should be called from another thread.
225215
std::future<std::string> future;
226216
try {
227-
future = event->internal_sync_->promise.get_future();
217+
future = event->handle_data_->promise.get_future();
228218
} catch (...) {
229219
TRACE(MSGPORT, "invalid promise");
230220
return Error::InvalidMessageEvent;

‎deps/message-port/message-port.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#pragma once
1818

1919
#include <functional>
20+
#include <future>
2021
#include <memory>
2122
#include <string>
2223
#include <vector>
@@ -39,7 +40,7 @@ class EXPORT_API MessageEvent {
3940
const std::vector<std::weak_ptr<Port>> ports() const;
4041
const std::weak_ptr<Port>& target() const;
4142

42-
virtual bool IsSync() { return false; }
43+
virtual bool IsSync() const { return false; }
4344

4445
virtual ~MessageEvent();
4546

@@ -56,15 +57,20 @@ class EXPORT_API MessageEventSync final : public MessageEvent {
5657
static std::shared_ptr<MessageEventSync> New(const std::string& data);
5758
~MessageEventSync();
5859

59-
bool IsSync() override { return true; }
60+
struct HandleData {
61+
HandleData();
62+
~HandleData();
63+
std::promise<std::string> promise;
64+
};
65+
66+
bool IsSync() const override { return true; }
6067

61-
void SetResult(const std::string& result) const;
68+
std::shared_ptr<HandleData> handle_data() const { return handle_data_; }
6269

6370
private:
6471
MessageEventSync(const std::string& data);
6572

66-
struct Internal;
67-
std::unique_ptr<Internal> internal_sync_;
73+
std::shared_ptr<HandleData> handle_data_;
6874
friend class Port;
6975
};
7076

‎include/lwnode/expected.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ class Expected {
5656
Expected(T&& v) : value_(std::move(v)), has_value_(true) {}
5757
Expected(const E& e) : error_(e), has_value_(false) {}
5858
Expected(E&& e) : error_(std::move(e)), has_value_(false) {}
59-
~Expected() { has_value_ ? value_.~T() : error_.~E(); }
59+
~Expected() {
60+
if (!has_value_) error_.~E();
61+
}
6062

6163
bool valid() const { return has_value_; }
6264
T& value() { return value_; }

‎include/lwnode/message-port.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#pragma once
1818

1919
#include <functional>
20+
#include <future>
2021
#include <memory>
2122
#include <string>
2223
#include <vector>
@@ -39,7 +40,7 @@ class EXPORT_API MessageEvent {
3940
const std::vector<std::weak_ptr<Port>> ports() const;
4041
const std::weak_ptr<Port>& target() const;
4142

42-
virtual bool IsSync() { return false; }
43+
virtual bool IsSync() const { return false; }
4344

4445
virtual ~MessageEvent();
4546

@@ -56,15 +57,20 @@ class EXPORT_API MessageEventSync final : public MessageEvent {
5657
static std::shared_ptr<MessageEventSync> New(const std::string& data);
5758
~MessageEventSync();
5859

59-
bool IsSync() override { return true; }
60+
struct HandleData {
61+
HandleData();
62+
~HandleData();
63+
std::promise<std::string> promise;
64+
};
65+
66+
bool IsSync() const override { return true; }
6067

61-
void SetResult(const std::string& result) const;
68+
std::shared_ptr<HandleData> handle_data() const { return handle_data_; }
6269

6370
private:
6471
MessageEventSync(const std::string& data);
6572

66-
struct Internal;
67-
std::unique_ptr<Internal> internal_sync_;
73+
std::shared_ptr<HandleData> handle_data_;
6874
friend class Port;
6975
};
7076

‎src/lwnode/nd-mod-message-port.cc‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -76,37 +76,49 @@ static ObjectRef* InstantiateMessageEvent(ExecutionStateRef* state,
7676

7777
// Create a new MessageEvent
7878
auto option = ObjectRef::create(state);
79-
option->setExtraData((void*)(event));
79+
80+
struct MessageEventExtraData : public gc {
81+
std::shared_ptr<MessageEventSync::HandleData> handle;
82+
};
83+
84+
if (event->IsSync()) {
85+
auto* data = new MessageEventExtraData();
86+
auto* event_sync =
87+
reinterpret_cast<MessageEventSync*>(const_cast<MessageEvent*>(event));
88+
data->handle = event_sync->handle_data();
89+
option->setExtraData(data);
90+
}
91+
8092
// TODO: Use atomic string
8193
option->set(state, OneByteString("data"), OneByteString(event->data()));
8294
option->set(state, OneByteString("origin"), OneByteString(event->origin()));
8395

84-
SetMethod(state,
85-
option,
86-
"setResult",
87-
[](ExecutionStateRef* state,
88-
ValueRef* this_value,
89-
size_t argc,
90-
ValueRef** argv,
91-
bool is_construct) -> ValueRef* {
92-
if (argc < 1 || !argv[0]->isString()) {
93-
state->throwException(TypeErrorObjectRef::create(
94-
state, OneByteString("Invalid argument")));
95-
}
96-
97-
auto event = GetExtraData<MessageEvent>(this_value);
98-
if (!event->IsSync()) {
99-
state->throwException(ErrorObjectRef::create(
100-
state,
101-
ErrorObjectRef::Code::None,
102-
OneByteString("only support MessageEventSync")));
103-
}
104-
105-
reinterpret_cast<MessageEventSync*>(event)->SetResult(
106-
argv[0]->asString()->toStdUTF8String());
107-
108-
return ValueRef::createUndefined();
109-
});
96+
SetMethod(
97+
state,
98+
option,
99+
"setResult",
100+
[](ExecutionStateRef* state,
101+
ValueRef* this_value,
102+
size_t argc,
103+
ValueRef** argv,
104+
bool is_construct) -> ValueRef* {
105+
if (argc < 1 || !argv[0]->isString()) {
106+
state->throwException(TypeErrorObjectRef::create(
107+
state, OneByteString("Invalid argument")));
108+
}
109+
110+
if (!this_value->asObject()->extraData()) {
111+
state->throwException(ErrorObjectRef::create(
112+
state,
113+
ErrorObjectRef::Code::None,
114+
OneByteString("only support MessageEventSync")));
115+
}
116+
117+
auto* data = GetExtraData<MessageEventExtraData>(this_value);
118+
data->handle->promise.set_value(argv[0]->asString()->toStdUTF8String());
119+
120+
return ValueRef::createUndefined();
121+
});
110122

111123
ValueRef* argv[] = {OneByteString("message"), option};
112124
return klass->construct(state, COUNT_OF(argv), argv)->asObject();

0 commit comments

Comments
 (0)