Skip to content

Commit 599a195

Browse files
hendrikmuhshendrikmuhs
authored
bug: fix crash if Compile gets called twice (#319)
prevent a double free crash if Compile gets called more than once
1 parent e7892c0 commit 599a195

File tree

5 files changed

+67
-43
lines changed

5 files changed

+67
-43
lines changed

‎keyvi/CPPLINT.cfg‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
linelength=120
22
root=include
3-
filter=-build/include_subdir
3+
filter=-build/include_subdir,-whitespace/indent_namespace

‎keyvi/include/keyvi/dictionary/dictionary_compiler.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ class DictionaryCompiler final {
129129
* Do the final compilation
130130
*/
131131
void Compile(callback_t progress_callback = nullptr, void* user_data = nullptr) {
132+
// already compiled
133+
if (generator_) {
134+
return;
135+
}
136+
132137
value_store_->CloseFeeding();
133138

134139
if (chunk_ == 0) {

‎keyvi/include/keyvi/dictionary/dictionary_index_compiler.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ class DictionaryIndexCompiler final {
126126
* Do the final compilation
127127
*/
128128
void Compile() {
129+
// already compiled
130+
if (generator_) {
131+
return;
132+
}
133+
129134
value_store_->CloseFeeding();
130135
Sort();
131136

‎keyvi/tests/.clang-tidy‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
# be less strict for tests
33
InheritParentConfig: true
44
Checks: "-cppcoreguidelines-avoid-magic-numbers,
5+
-misc-include-cleaner,
56
-readability-function-cognitive-complexity,
67
-readability-function-cognitive-complexity,
78
-readability-identifier-length,
89
-readability-magic-numbers,
9-
"
10+
"

‎keyvi/tests/keyvi/dictionary/dictionary_compiler_test.cpp‎

Lines changed: 54 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
* Author: hendrik
2424
*/
2525

26+
#include <string>
27+
#include <vector>
28+
2629
#include <boost/test/unit_test.hpp>
2730

2831
#include "keyvi/dictionary/dictionary.h"
@@ -48,7 +51,7 @@ class DCTTestHelper final {
4851
static uint32_t GetStateIdForPrefix(const fsa::automata_t& fsa, const std::string& query) {
4952
uint32_t state = fsa->GetStartState();
5053
size_t depth = 0;
51-
size_t query_length = query.size();
54+
const size_t query_length = query.size();
5255

5356
while (state != 0 && depth != query_length) {
5457
state = fsa->TryWalkTransition(state, query[depth]);
@@ -59,13 +62,13 @@ class DCTTestHelper final {
5962
}
6063
};
6164

62-
typedef boost::mpl::list<keyvi::dictionary::DictionaryCompiler<dictionary_type_t::INT_WITH_WEIGHTS>,
63-
keyvi::dictionary::DictionaryIndexCompiler<dictionary_type_t::INT_WITH_WEIGHTS>>
64-
int_with_weight_types;
65+
using int_with_weight_types =
66+
boost::mpl::list<keyvi::dictionary::DictionaryCompiler<dictionary_type_t::INT_WITH_WEIGHTS>,
67+
keyvi::dictionary::DictionaryIndexCompiler<dictionary_type_t::INT_WITH_WEIGHTS>>;
6568

6669
BOOST_AUTO_TEST_CASE_TEMPLATE(minimizationIntInnerWeights, DictT, int_with_weight_types) {
6770
// simulating permutation
68-
std::vector<std::pair<std::string, uint32_t>> test_data = {
71+
const std::vector<std::pair<std::string, uint32_t>> test_data = {
6972
{"fb#fb msg downl de", 22}, {"msg#fb msg downl de", 22}, {"downl#fb msg downl de", 22},
7073
{"de#fb msg downl de", 22}, {"fb msg#fb msg downl de", 22}, {"fb downl#fb msg downl de", 22},
7174
{"fb de#fb msg downl de", 22}, {"msg fb#fb msg downl de", 22}, {"msg downl#fb msg downl de", 22},
@@ -76,62 +79,63 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(minimizationIntInnerWeights, DictT, int_with_weigh
7679

7780
DictT compiler(keyvi::util::parameters_t({{"memory_limit_mb", "10"}}));
7881

79-
for (auto p : test_data) {
82+
for (const auto& p : test_data) {
8083
compiler.Add(p.first, p.second);
8184
}
8285
compiler.Compile();
8386

8487
boost::filesystem::path temp_path = boost::filesystem::temp_directory_path();
8588

8689
temp_path /= boost::filesystem::unique_path("dictionary-unit-test-dictionarycompiler-%%%%-%%%%-%%%%-%%%%");
87-
std::string file_name = temp_path.string();
90+
const std::string file_name = temp_path.string();
8891

8992
compiler.WriteToFile(file_name);
9093

91-
Dictionary d(file_name.c_str());
92-
fsa::automata_t a = d.GetFsa();
94+
const Dictionary d(file_name);
95+
const fsa::automata_t a = d.GetFsa();
9396

94-
std::string reference_value("de#");
95-
uint32_t reference_weight = DCTTestHelper::GetStateIdForPrefix(a, reference_value);
97+
const std::string reference_value("de#");
98+
const uint32_t reference_weight = DCTTestHelper::GetStateIdForPrefix(a, reference_value);
9699

97-
std::vector<std::string> query_data = {"fb#", "msg#", "downl#", "fb msg#", "fb downl#", "downl fb#", "downl de#"};
100+
const std::vector<std::string> query_data = {"fb#", "msg#", "downl#", "fb msg#",
101+
"fb downl#", "downl fb#", "downl de#"};
98102

99103
size_t tested_values = 0;
100-
for (auto q : query_data) {
104+
for (const auto& q : query_data) {
101105
BOOST_CHECK(reference_weight == DCTTestHelper::GetStateIdForPrefix(a, q));
102106
++tested_values;
103107
}
104108

105109
BOOST_CHECK(tested_values == query_data.size());
106-
std::remove(file_name.c_str());
110+
BOOST_CHECK(std::remove(file_name.c_str()) == 0);
107111
}
108112

109113
BOOST_AUTO_TEST_CASE_TEMPLATE(unsortedKeys, DictT, int_with_weight_types) {
110114
// simulating permutation
111-
std::vector<std::pair<std::string, uint32_t>> test_data = {
115+
const std::vector<std::pair<std::string, uint32_t>> test_data = {
112116
{"uboot", 22}, {"überfall", 33}, {"vielleicht", 43}, {"arbeit", 3}, {"zoo", 5}, {"ändern", 6},
113117
};
114118

115119
DictT compiler(keyvi::util::parameters_t({{"memory_limit_mb", "10"}}));
116120

117-
for (auto p : test_data) {
121+
for (const auto& p : test_data) {
118122
compiler.Add(p.first, p.second);
119123
}
120124
compiler.Compile();
121125

122126
boost::filesystem::path temp_path = boost::filesystem::temp_directory_path();
123127

124128
temp_path /= boost::filesystem::unique_path("dictionary-unit-test-dictionarycompiler-%%%%-%%%%-%%%%-%%%%");
125-
std::string file_name = temp_path.string();
129+
const std::string file_name = temp_path.string();
126130

127131
compiler.WriteToFile(file_name);
128132

129-
Dictionary d(file_name.c_str());
133+
const Dictionary d(file_name);
130134

131-
fsa::automata_t f(d.GetFsa());
135+
const fsa::automata_t f(d.GetFsa());
132136

133137
fsa::EntryIterator it(f);
134-
fsa::EntryIterator end_it;
138+
const fsa::EntryIterator end_it;
135139

136140
std::stringstream ss;
137141

@@ -178,35 +182,35 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(unsortedKeys, DictT, int_with_weight_types) {
178182
++it;
179183
BOOST_CHECK(it == end_it);
180184

181-
std::remove(file_name.c_str());
185+
BOOST_CHECK(std::remove(file_name.c_str()) == 0);
182186
}
183187

184188
BOOST_AUTO_TEST_CASE_TEMPLATE(compactSize, DictT, int_with_weight_types) {
185189
// simulating permutation
186-
std::vector<std::pair<std::string, uint32_t>> test_data = {
190+
const std::vector<std::pair<std::string, uint32_t>> test_data = {
187191
{"uboot", 22}, {"überfall", 33}, {"vielleicht", 43}, {"arbeit", 3}, {"zoo", 5}, {"ändern", 6},
188192
};
189193

190194
DictT compiler(keyvi::util::parameters_t({{"memory_limit_mb", "10"}}));
191195

192-
for (auto p : test_data) {
196+
for (const auto& p : test_data) {
193197
compiler.Add(p.first, p.second);
194198
}
195199
compiler.Compile();
196200

197201
boost::filesystem::path temp_path = boost::filesystem::temp_directory_path();
198202

199203
temp_path /= boost::filesystem::unique_path("dictionary-unit-test-dictionarycompiler-%%%%-%%%%-%%%%-%%%%");
200-
std::string file_name = temp_path.string();
204+
const std::string file_name = temp_path.string();
201205

202206
compiler.WriteToFile(file_name);
203207

204-
Dictionary d(file_name.c_str());
208+
const Dictionary d(file_name);
205209

206-
fsa::automata_t f(d.GetFsa());
210+
const fsa::automata_t f(d.GetFsa());
207211

208212
fsa::EntryIterator it(f);
209-
fsa::EntryIterator end_it;
213+
const fsa::EntryIterator end_it;
210214

211215
BOOST_CHECK_EQUAL("arbeit", it.GetKey());
212216
++it;
@@ -222,12 +226,11 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(compactSize, DictT, int_with_weight_types) {
222226
++it;
223227
BOOST_CHECK(it == end_it);
224228

225-
std::remove(file_name.c_str());
229+
BOOST_CHECK(std::remove(file_name.c_str()) == 0);
226230
}
227231

228-
typedef boost::mpl::list<keyvi::dictionary::DictionaryCompiler<dictionary_type_t::JSON>,
229-
keyvi::dictionary::DictionaryIndexCompiler<dictionary_type_t::JSON>>
230-
json_types;
232+
using json_types = boost::mpl::list<keyvi::dictionary::DictionaryCompiler<dictionary_type_t::JSON>,
233+
keyvi::dictionary::DictionaryIndexCompiler<dictionary_type_t::JSON>>;
231234

232235
BOOST_AUTO_TEST_CASE_TEMPLATE(MultipleKeyUpdateAndCompile, DictT, json_types) {
233236
DictT compiler(keyvi::util::parameters_t({{"memory_limit_mb", "10"}}));
@@ -248,14 +251,14 @@ void bigger_compile_test(const keyvi::util::parameters_t& params = keyvi::util::
248251

249252
boost::filesystem::path temp_path = boost::filesystem::temp_directory_path();
250253
temp_path /= boost::filesystem::unique_path("dictionary-unit-test-dictionarycompiler-%%%%-%%%%-%%%%-%%%%");
251-
std::string file_name = temp_path.string();
254+
const std::string file_name = temp_path.string();
252255

253256
compiler.WriteToFile(file_name);
254257

255-
Dictionary d(file_name.c_str());
258+
const Dictionary d(file_name);
256259

257260
for (size_t i = 0; i < keys; ++i) {
258-
keyvi::dictionary::match_t m = d["loooooooooooooooonnnnnnnngggggggg_key-" + std::to_string(i)];
261+
const keyvi::dictionary::match_t m = d["loooooooooooooooonnnnnnnngggggggg_key-" + std::to_string(i)];
259262
BOOST_CHECK_EQUAL("loooooooooooooooonnnnnnnngggggggg_key-" + std::to_string(i), m->GetMatchedString());
260263
BOOST_CHECK_EQUAL("{\"id\":" + std::to_string(i) + "}", m->GetValueAsString());
261264
}
@@ -283,25 +286,35 @@ BOOST_AUTO_TEST_CASE(float_dictionary) {
283286
boost::filesystem::path temp_path = boost::filesystem::temp_directory_path();
284287

285288
temp_path /= boost::filesystem::unique_path("dictionary-unit-test-dictionarycompiler-%%%%-%%%%-%%%%-%%%%");
286-
std::string file_name = temp_path.string();
289+
const std::string file_name = temp_path.string();
287290

288291
compiler.WriteToFile(file_name);
289292

290-
Dictionary d(file_name.c_str());
293+
const Dictionary d(file_name);
291294

292295
bool matched = false;
293-
for (auto m : d.Get("abbe")) {
296+
for (const auto& m : d.Get("abbe")) {
294297
BOOST_CHECK_EQUAL("3.1, 0.2, 1.3, 0.4, 0.5", m->GetValueAsString());
295298
std::vector<float> float_vector = keyvi::util::DecodeFloatVector(m->GetRawValueAsString());
296299
BOOST_CHECK_EQUAL(5, float_vector.size());
297-
BOOST_CHECK_EQUAL(3.1f, float_vector[0]);
298-
BOOST_CHECK_EQUAL(1.3f, float_vector[2]);
299-
BOOST_CHECK_EQUAL(0.5f, float_vector[4]);
300+
BOOST_CHECK_EQUAL(3.1F, float_vector[0]);
301+
BOOST_CHECK_EQUAL(1.3F, float_vector[2]);
302+
BOOST_CHECK_EQUAL(0.5F, float_vector[4]);
300303
matched = true;
301304
}
302305
BOOST_CHECK(matched);
303306

304-
std::remove(file_name.c_str());
307+
BOOST_CHECK(std::remove(file_name.c_str()) == 0);
308+
}
309+
310+
BOOST_AUTO_TEST_CASE_TEMPLATE(MultipleCompile, DictT, json_types) {
311+
DictT compiler(keyvi::util::parameters_t({{"memory_limit_mb", "10"}}));
312+
313+
compiler.Add("a", "1");
314+
compiler.Add("b", "1");
315+
compiler.Compile();
316+
compiler.Compile();
317+
compiler.Compile();
305318
}
306319

307320
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)