Skip to content

Commit 1d0c259

Browse files
committed
fix: fixes for various errors related to shared_from_this.
A while back there were some changes to the pattern language library that changed the way shared_pointers are created using shared_from_this(). Unfortunatelly the changes were not complete and various bugs were created among them 2234, json type not working, unable to export files, static arrays of bitfields,... The cause of the errors was that in class Pattern the member m_parent was left as a raw pointer and it needs to be handled by shared pointers. Also there were some cases in which share pointers were needed but unique pointers were used instead. Both cause crashes when shared_from_this is used on pointers that are not managed by shared_ptr. Another source of errors were infinite loops of clone and reference that caused stack overflow. The fixes include making m_parent a weak pointer, turning unique pointers into shared pointers and moving codefrom the copy constructors into clone to break the infinite loops.These changes are the bare minimum needed to bring the pattern language back to the full functionality that it had before shared_from_this was introduced or at least thats the hope.
1 parent 7a741ce commit 1d0c259

25 files changed

Lines changed: 94 additions & 90 deletions

lib/include/pl/api.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ namespace pl::api {
126126
/**
127127
* @brief A function callback called when a custom built-in type is being instantiated
128128
*/
129-
using TypeCallback = std::function<std::unique_ptr<ptrn::Pattern>(core::Evaluator *, const std::vector<core::Token::Literal> &)>;
129+
using TypeCallback = std::function<std::shared_ptr<ptrn::Pattern>(core::Evaluator *, const std::vector<core::Token::Literal> &)>;
130130

131131
/**
132132
* @brief A type representing a function.

lib/include/pl/patterns/pattern.hpp

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,13 @@ namespace pl::ptrn {
116116
}
117117

118118
virtual std::shared_ptr<Pattern> clone() const = 0;
119-
std::shared_ptr<Pattern> reference() { return shared_from_this(); }
119+
std::shared_ptr<Pattern> reference() {
120+
auto weakPtr = weak_from_this();
121+
if (weakPtr.expired()) {
122+
core::err::E0001.throwError("Cannot call shared_from_this if this is not shared.");
123+
}
124+
return shared_from_this();
125+
}
120126

121127
[[nodiscard]] u64 getOffset() const { return this->m_offset; }
122128
[[nodiscard]] virtual u128 getOffsetForSorting() const { return this->getOffset() << 3; }
@@ -537,15 +543,15 @@ namespace pl::ptrn {
537543
this->m_initialized = initialized;
538544
}
539545

540-
[[nodiscard]] const Pattern* getParent() const {
541-
return m_parent;
546+
[[nodiscard]] const std::shared_ptr<Pattern> getParent() const {
547+
return m_parent.lock();
542548
}
543549

544-
[[nodiscard]] Pattern* getParent() {
545-
return m_parent;
550+
[[nodiscard]] std::shared_ptr<Pattern> getParent() {
551+
return m_parent.lock();
546552
}
547553

548-
void setParent(Pattern *parent) {
554+
void setParent(std::shared_ptr<Pattern> parent) {
549555
m_parent = parent;
550556
}
551557

@@ -623,7 +629,7 @@ namespace pl::ptrn {
623629
core::Evaluator *m_evaluator;
624630

625631
std::unique_ptr<std::map<std::string, std::vector<core::Token::Literal>>> m_attributes;
626-
Pattern *m_parent = nullptr;
632+
std::weak_ptr<Pattern> m_parent;
627633
u32 m_line = 0;
628634

629635
std::set<std::string>::const_iterator m_variableName;

lib/include/pl/patterns/pattern_array_dynamic.hpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,18 @@ namespace pl::ptrn {
88
public IInlinable,
99
public IIndexable {
1010
public:
11-
PatternArrayDynamic(core::Evaluator *evaluator, u64 offset, size_t size, u32 line)
12-
: Pattern(evaluator, offset, size, line) { }
11+
PatternArrayDynamic(core::Evaluator *evaluator, u64 offset, size_t size, u32 line) : Pattern(evaluator, offset, size, line) { }
1312

14-
PatternArrayDynamic(const PatternArrayDynamic &other) : Pattern(other) {
15-
std::vector<std::shared_ptr<Pattern>> entries;
16-
for (const auto &entry : other.m_entries)
17-
entries.push_back(entry->clone());
18-
19-
this->setEntries(entries);
20-
}
13+
PatternArrayDynamic(const PatternArrayDynamic &other) : Pattern(other) {}
2114

2215
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
23-
return std::unique_ptr<Pattern>(new PatternArrayDynamic(*this));
16+
auto other = std::make_shared<PatternArrayDynamic>(*this);
17+
std::vector<std::shared_ptr<Pattern>> entries;
18+
for (const auto &entry : this->m_entries) {
19+
entries.push_back(entry->clone());
20+
}
21+
other->setEntries(entries);
22+
return other;
2423
}
2524

2625
void setColor(u32 color) override {
@@ -133,7 +132,7 @@ namespace pl::ptrn {
133132

134133
if (!entry->hasOverriddenColor())
135134
entry->setBaseColor(this->getColor());
136-
entry->setParent(this);
135+
entry->setParent(reference());
137136

138137
this->m_entries.emplace_back(entry);
139138
}

lib/include/pl/patterns/pattern_array_static.hpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ namespace pl::ptrn {
1111
PatternArrayStatic(core::Evaluator *evaluator, u64 offset, size_t size, u32 line)
1212
: Pattern(evaluator, offset, size, line) { }
1313

14-
PatternArrayStatic(const PatternArrayStatic &other) : Pattern(other) {
15-
this->setEntries(other.getTemplate()->clone(), other.getEntryCount());
16-
}
14+
PatternArrayStatic(const PatternArrayStatic &other) : Pattern(other) {}
1715

1816
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
19-
return std::unique_ptr<Pattern>(new PatternArrayStatic(*this));
17+
auto other = std::make_shared<PatternArrayStatic>(*this);
18+
other->setEntries(this->m_template->clone(), this->m_entryCount);
19+
return other;
2020
}
2121

2222
[[nodiscard]] std::shared_ptr<Pattern> getEntry(size_t index) const override {
@@ -153,7 +153,7 @@ namespace pl::ptrn {
153153

154154
void setEntries(std::shared_ptr<Pattern> &&templatePattern, size_t count) {
155155
this->m_template = std::move(templatePattern);
156-
this->m_template->setParent(this);
156+
this->m_template->setParent(reference());
157157
this->m_highlightTemplates.push_back(this->m_template->clone());
158158
this->m_entryCount = count;
159159

lib/include/pl/patterns/pattern_bitfield.hpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace pl::ptrn {
1212
[[nodiscard]] const PatternBitfieldMember& getTopmostBitfield() const {
1313
const PatternBitfieldMember* topBitfield = this;
1414
while (auto parent = topBitfield->getParent()) {
15-
auto parentBitfield = dynamic_cast<const PatternBitfieldMember*>(parent);
15+
auto parentBitfield = dynamic_cast<const PatternBitfieldMember*>(parent.get());
1616
if (parentBitfield == nullptr)
1717
break;
1818

@@ -55,7 +55,8 @@ namespace pl::ptrn {
5555
public:
5656
PatternBitfieldField(core::Evaluator *evaluator, u64 offset, u8 bitOffset, u8 bitSize, u32 line, PatternBitfieldMember *parentBitfield = nullptr)
5757
: PatternBitfieldMember(evaluator, offset, (bitOffset + bitSize + 7) / 8, line), m_bitOffset(bitOffset % 8), m_bitSize(bitSize) {
58-
this->setParent(parentBitfield);
58+
if (parentBitfield != nullptr)
59+
this->setParent(parentBitfield->reference());
5960
}
6061

6162
PatternBitfieldField(const PatternBitfieldField &other) : PatternBitfieldMember(other) {
@@ -65,7 +66,7 @@ namespace pl::ptrn {
6566
}
6667

6768
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
68-
return std::unique_ptr<Pattern>(new PatternBitfieldField(*this));
69+
return std::make_shared<PatternBitfieldField>(*this);
6970
}
7071

7172
[[nodiscard]] u128 readValue() const {
@@ -162,7 +163,7 @@ namespace pl::ptrn {
162163
using PatternBitfieldField::PatternBitfieldField;
163164

164165
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
165-
return std::unique_ptr<Pattern>(new PatternBitfieldFieldSigned(*this));
166+
return std::make_shared<PatternBitfieldFieldSigned>(*this);
166167
}
167168

168169
[[nodiscard]] core::Token::Literal getValue() const override {
@@ -186,7 +187,7 @@ namespace pl::ptrn {
186187
using PatternBitfieldField::PatternBitfieldField;
187188

188189
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
189-
return std::unique_ptr<Pattern>(new PatternBitfieldFieldBoolean(*this));
190+
return std::make_shared<PatternBitfieldFieldBoolean>(*this);
190191
}
191192

192193
[[nodiscard]] core::Token::Literal getValue() const override {
@@ -245,7 +246,7 @@ namespace pl::ptrn {
245246
}
246247

247248
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
248-
return std::unique_ptr<Pattern>(new PatternBitfieldFieldEnum(*this));
249+
return std::make_shared<PatternBitfieldFieldEnum>(*this);
249250
}
250251

251252
std::string formatDisplayValue() override {
@@ -282,7 +283,7 @@ namespace pl::ptrn {
282283
}
283284

284285
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
285-
return std::unique_ptr<Pattern>(new PatternBitfieldArray(*this));
286+
return std::make_shared<PatternBitfieldArray>(*this);
286287
}
287288

288289
[[nodiscard]] u8 getBitOffset() const override {
@@ -422,7 +423,7 @@ namespace pl::ptrn {
422423
if (!entry->hasOverriddenColor())
423424
entry->setBaseColor(this->getColor());
424425

425-
entry->setParent(this);
426+
entry->setParent(reference());
426427

427428
this->m_sortedEntries.push_back(entry.get());
428429
}
@@ -554,7 +555,7 @@ namespace pl::ptrn {
554555
}
555556

556557
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
557-
return std::unique_ptr<Pattern>(new PatternBitfield(*this));
558+
return std::make_shared<PatternBitfield>(*this);
558559
}
559560

560561
[[nodiscard]] u8 getBitOffset() const override {
@@ -643,7 +644,7 @@ namespace pl::ptrn {
643644
this->setBaseColor(this->m_fields.front()->getColor());
644645

645646
for (const auto &field : this->m_fields) {
646-
field->setParent(this);
647+
field->setParent(reference());
647648
this->m_sortedFields.push_back(field.get());
648649
}
649650
}

lib/include/pl/patterns/pattern_boolean.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace pl::ptrn {
1010
: Pattern(evaluator, offset, 1, line) { }
1111

1212
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
13-
return std::unique_ptr<Pattern>(new PatternBoolean(*this));
13+
return std::make_shared<PatternBoolean>(*this);
1414
}
1515

1616
[[nodiscard]] core::Token::Literal getValue() const override {

lib/include/pl/patterns/pattern_character.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace pl::ptrn {
1010
: Pattern(evaluator, offset, 1, line) { }
1111

1212
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
13-
return std::make_unique<PatternCharacter>(*this);
13+
return std::make_shared<PatternCharacter>(*this);
1414
}
1515

1616
[[nodiscard]] core::Token::Literal getValue() const override {

lib/include/pl/patterns/pattern_enum.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace pl::ptrn {
1818
: Pattern(evaluator, offset, size, line) { }
1919

2020
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
21-
return std::unique_ptr<Pattern>(new PatternEnum(*this));
21+
return std::make_shared<PatternEnum>(*this);
2222
}
2323

2424
[[nodiscard]] core::Token::Literal getValue() const override {

lib/include/pl/patterns/pattern_error.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace pl::ptrn {
1010
: Pattern(evaluator, offset, size, line), m_errorMessage(std::move(errorMessage)) { }
1111

1212
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
13-
return std::unique_ptr<Pattern>(new PatternError(*this));
13+
return std::make_shared<PatternError>(*this);
1414
}
1515

1616
[[nodiscard]] std::string getFormattedName() const override {

lib/include/pl/patterns/pattern_float.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace pl::ptrn {
1010
: Pattern(evaluator, offset, size, line) { }
1111

1212
[[nodiscard]] std::shared_ptr<Pattern> clone() const override {
13-
return std::unique_ptr<Pattern>(new PatternFloat(*this));
13+
return std::make_shared<PatternFloat>(*this);
1414
}
1515

1616
[[nodiscard]] core::Token::Literal getValue() const override {

0 commit comments

Comments
 (0)