Skip to content

Commit

Permalink
fix: memory leak on api_status in flatbuffer_to_examples
Browse files Browse the repository at this point in the history
* also fixes up some leaks / warnings in the tests using deprecated APIs
  • Loading branch information
lokitoth committed Jan 19, 2024
1 parent 44d01b0 commit 7a4b013
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
7 changes: 5 additions & 2 deletions vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ namespace flatbuffer
{
int flatbuffer_to_examples(VW::workspace* all, io_buf& buf, VW::multi_ex& examples)
{
// if (all->options->was_supplied("api_status"))
if (all->parser_runtime.api_status)
{
// TODO: At what point do we report the error?
VW::experimental::api_status status;
return static_cast<int>(all->parser_runtime.flat_converter->parse_examples(all, buf, examples, nullptr,
new VW::experimental::api_status()) == VW::experimental::error_code::success);
&status) == VW::experimental::error_code::success);
}
else
return static_cast<int>(all->parser_runtime.flat_converter->parse_examples(all, buf, examples, nullptr, nullptr) ==
VW::experimental::error_code::success);
Expand Down
23 changes: 11 additions & 12 deletions vowpalwabbit/fb_parser/tests/flatbuffer_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_audit)
// Check vw example
EXPECT_EQ(examples.size(), 1);
EXPECT_FLOAT_EQ(examples[0]->l.simple.label, 0.f);
const auto& red_features = examples[0]->ex_reduction_features.template get<simple_label_reduction_features>();
const auto& red_features = examples[0]->ex_reduction_features.template get<VW::simple_label_reduction_features>();
EXPECT_FLOAT_EQ(red_features.weight, 1.f);

EXPECT_EQ(examples[0]->indices[0], VW::details::CONSTANT_NAMESPACE);
Expand All @@ -137,7 +137,7 @@ TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_audit)
TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_no_audit)
{
// Testcase where user would provide feature names and feature values (no feature hashes)
auto all = VW::initialize("--no_stdin --quiet --flatbuffer", nullptr, false, nullptr, nullptr);
auto all = VW::initialize(vwtest::make_args("--no_stdin", "--quiet", "--flatbuffer"));

flatbuffers::FlatBufferBuilder builder;

Expand All @@ -147,9 +147,9 @@ TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_no_audit)
uint8_t* buf = builder.GetBufferPointer();

VW::multi_ex examples;
examples.push_back(&VW::get_unused_example(all));
io_buf unused_buffer;
all->parser_runtime.flat_converter->parse_examples(all, unused_buffer, examples, buf);
examples.push_back(&VW::get_unused_example(all.get()));
VW::io_buf unused_buffer;
all->parser_runtime.flat_converter->parse_examples(all.get(), unused_buffer, examples, buf);

auto example = all->parser_runtime.flat_converter->data()->example_obj_as_Example();
EXPECT_EQ(example->namespaces()->size(), 1);
Expand Down Expand Up @@ -224,7 +224,7 @@ TEST(FlatbufferParser, FlatbufferCollection)
TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_error_code)
{
// Testcase where user would provide feature names and feature values (no feature hashes)
auto all = VW::initialize("--no_stdin --quiet --flatbuffer --api_status --audit", nullptr, false, nullptr, nullptr);
auto all = VW::initialize(vwtest::make_args("--no_stdin", "--quiet", "--flatbuffer", "--api_status", "--audit"));

flatbuffers::FlatBufferBuilder builder;

Expand All @@ -234,10 +234,10 @@ TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_error_code)
uint8_t* buf = builder.GetBufferPointer();

VW::multi_ex examples;
examples.push_back(&VW::get_unused_example(all));
io_buf unused_buffer;
EXPECT_EQ(all->parser_runtime.flat_converter->parse_examples(all, unused_buffer, examples, buf), 8);
EXPECT_EQ(all->parser_runtime.example_parser->reader(all, unused_buffer, examples), 0);
examples.push_back(&VW::get_unused_example(all.get()));
VW::io_buf unused_buffer;
EXPECT_EQ(all->parser_runtime.flat_converter->parse_examples(all.get(), unused_buffer, examples, buf), 8);
EXPECT_EQ(all->parser_runtime.example_parser->reader(all.get(), unused_buffer, examples), 0);

auto example = all->parser_runtime.flat_converter->data()->example_obj_as_Example();
EXPECT_EQ(example->namespaces()->size(), 1);
Expand All @@ -251,10 +251,9 @@ TEST(flatbuffer_parser_tests, test_flatbuffer_standalone_example_error_code)
// Check vw example
EXPECT_EQ(examples.size(), 1);
EXPECT_FLOAT_EQ(examples[0]->l.simple.label, 0.f);
const auto& red_features = examples[0]->ex_reduction_features.template get<simple_label_reduction_features>();
const auto& red_features = examples[0]->ex_reduction_features.template get<VW::simple_label_reduction_features>();
EXPECT_FLOAT_EQ(red_features.weight, 1.f);
EXPECT_EQ(examples[0]->indices[0], VW::details::CONSTANT_NAMESPACE);

VW::finish_example(*all, *examples[0]);
VW::finish(*all);
}

0 comments on commit 7a4b013

Please sign in to comment.