[Backport release-2.23] Fix segfaults in WebP queries ran in parallel. (#5065)

test/support/src/whitebox_helpers.h

@@ -0,0 +1,48 @@
+/**
+ * @file whitebox_helpers.h
+ *
+ * @section LICENSE
+ *
+ * The MIT License
+ *
+ * @copyright Copyright (c) 2024 TileDB, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @section DESCRIPTION
+ *
+ * Helpers to provide whitebox access to TileDB internals for testing.
+ */
+#ifndef TILEDB_WHITEBOX_HELPERS_H
+#define TILEDB_WHITEBOX_HELPERS_H
+
+#include "tiledb/sm/tile/tile.h"
+
+namespace tiledb::sm {
+
+class WhiteboxWriterTile {
+ public:
+  static void set_max_tile_chunk_size(uint64_t size) {
+    WriterTile::max_tile_chunk_size_ = size;
+  }
+};
+
+}  // namespace tiledb::sm
+
+#endif  // TILEDB_WHITEBOX_HELPERS_H

tiledb/sm/filter/filter_pipeline.cc

@@ -45,6 +45,7 @@
 #include "tiledb/sm/misc/parallel_functions.h"
 #include "tiledb/sm/stats/global_stats.h"
 #include "tiledb/sm/tile/tile.h"
+#include "webp_filter.h"
 
 using namespace tiledb::common;
 
@@ -644,6 +645,8 @@ bool FilterPipeline::use_tile_chunking(
     } else if (version >= 13 && has_filter(FilterType::FILTER_DICTIONARY)) {
       return false;
     }
+  } else if (has_filter(FilterType::FILTER_WEBP)) {
+    return false;
   }
 
   return true;

tiledb/sm/filter/test/CMakeLists.txt

@@ -54,6 +54,7 @@ commence(unit_test run_filter_pipeline)
         unit_encryption_pipeline.cc
         unit_positive_delta_pipeline.cc
         unit_run_filter_pipeline.cc
+        unit_webp_pipeline.cc
         unit_xor_pipeline.cc
     )
 conclude(unit_test)

tiledb/sm/filter/test/filter_test_support.cc

@@ -42,12 +42,12 @@ SimpleVariableTestData::SimpleVariableTestData()
     : target_ncells_per_chunk_{10}
     , elements_per_chunk_{14, 6, 11, 7, 10, 10, 20, 10, 12}
     , tile_data_generator_{{4, 10, 6, 11, 7, 9, 1, 10, 20, 2, 2, 2, 2, 2, 12}} {
-  WriterTile::set_max_tile_chunk_size(
+  WhiteboxWriterTile::set_max_tile_chunk_size(
       target_ncells_per_chunk_ * sizeof(uint64_t));
 }
 
 SimpleVariableTestData::~SimpleVariableTestData() {
-  WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
 }
 
 /**

tiledb/sm/filter/test/tile_data_generator.h

@@ -37,6 +37,7 @@
 #include <algorithm>
 #include <numeric>
 #include <optional>
+#include "test/support/src/whitebox_helpers.h"
 #include "tiledb/sm/tile/tile.h"
 
 using namespace tiledb::common;
@@ -139,7 +140,7 @@ class IncrementTileDataGenerator : public TileDataGenerator {
   }
 
   ~IncrementTileDataGenerator() {
-    WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+    WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
   }
 
   uint64_t cell_size() const override {

tiledb/sm/filter/test/unit_bit_width_reduction_pipeline.cc

@@ -269,7 +269,7 @@ TEST_CASE(
     auto tile = make_increasing_tile(nelts, tracker);
     auto offsets_tile = make_offsets_tile(offsets, tracker);
 
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     CHECK(
         pipeline.run_forward(&dummy_stats, tile.get(), offsets_tile.get(), &tp)
             .ok());
@@ -335,7 +335,7 @@ TEST_CASE(
   }
 
   SECTION("- Window sizes") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     std::vector<uint32_t> window_sizes = {
         32, 64, 128, 256, 437, 512, 1024, 2000};
     for (auto window_size : window_sizes) {
@@ -362,7 +362,7 @@ TEST_CASE(
   }
 
   SECTION("- Random values") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     std::random_device rd;
     auto seed = rd();
     std::mt19937 gen(seed), gen_copy(seed);
@@ -401,7 +401,7 @@ TEST_CASE(
   }
 
   SECTION(" - Random signed values") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     std::random_device rd;
     auto seed = rd();
     std::mt19937 gen(seed), gen_copy(seed);
@@ -460,7 +460,7 @@ TEST_CASE(
   }
 
   SECTION("- Byte overflow") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     auto tile = make_shared<WriterTile>(
         HERE(),
         constants::format_version,
@@ -493,5 +493,5 @@ TEST_CASE(
     }
   }
 
-  WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
 }

tiledb/sm/filter/test/unit_bitshuffle_pipeline.cc

@@ -143,7 +143,7 @@ TEST_CASE("Filter: Test bitshuffle var", "[filter][bitshuffle][var]") {
   pipeline.add_filter(BitshuffleFilter(Datatype::UINT64));
 
   SECTION("- Single stage") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     CHECK(
         pipeline.run_forward(&dummy_stats, tile.get(), offsets_tile.get(), &tp)
             .ok());
@@ -161,7 +161,7 @@ TEST_CASE("Filter: Test bitshuffle var", "[filter][bitshuffle][var]") {
   }
 
   SECTION("- Indivisible by 8") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     const uint32_t nelts2 = 1001;
     const uint64_t tile_size2 = nelts2 * sizeof(uint32_t);
 
@@ -194,5 +194,5 @@ TEST_CASE("Filter: Test bitshuffle var", "[filter][bitshuffle][var]") {
     }
   }
 
-  WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
 }

tiledb/sm/filter/test/unit_byteshuffle_pipeline.cc

@@ -141,7 +141,7 @@ TEST_CASE("Filter: Test byteshuffle var", "[filter][byteshuffle][var]") {
   pipeline.add_filter(ByteshuffleFilter(Datatype::UINT64));
 
   SECTION("- Single stage") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     CHECK(
         pipeline.run_forward(&dummy_stats, tile.get(), offsets_tile.get(), &tp)
             .ok());
@@ -159,7 +159,7 @@ TEST_CASE("Filter: Test byteshuffle var", "[filter][byteshuffle][var]") {
   }
 
   SECTION("- Uneven number of elements") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     const uint32_t nelts2 = 1001;
     const uint64_t tile_size2 = nelts2 * sizeof(uint32_t);
 
@@ -192,5 +192,5 @@ TEST_CASE("Filter: Test byteshuffle var", "[filter][byteshuffle][var]") {
     }
   }
 
-  WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
 }

tiledb/sm/filter/test/unit_positive_delta_pipeline.cc

@@ -173,7 +173,7 @@ TEST_CASE(
     auto tile = make_increasing_tile(nelts, tracker);
     auto offsets_tile = make_offsets_tile(offsets, tracker);
 
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     CHECK(
         pipeline.run_forward(&dummy_stats, tile.get(), offsets_tile.get(), &tp)
             .ok());
@@ -240,7 +240,7 @@ TEST_CASE(
   }
 
   SECTION("- Window sizes") {
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     std::vector<uint32_t> window_sizes = {
         32, 64, 128, 256, 437, 512, 1024, 2000};
     for (auto window_size : window_sizes) {
@@ -271,7 +271,7 @@ TEST_CASE(
     auto tile = make_increasing_tile(nelts, tracker);
     auto offsets_tile = make_offsets_tile(offsets, tracker);
 
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     for (uint64_t i = 0; i < nelts; i++) {
       auto val = nelts - i;
       CHECK_NOTHROW(tile->write(&val, i * sizeof(uint64_t), sizeof(uint64_t)));
@@ -282,5 +282,5 @@ TEST_CASE(
              .ok());
   }
 
-  WriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
 }

tiledb/sm/filter/test/unit_run_filter_pipeline.cc

@@ -293,7 +293,7 @@ TEST_CASE(
 
   SECTION("- Multi-stage") {
     // Add a few more +1 filters and re-run.
-    WriterTile::set_max_tile_chunk_size(80);
+    WhiteboxWriterTile::set_max_tile_chunk_size(80);
     pipeline.add_filter(Add1InPlace(Datatype::UINT64));
     pipeline.add_filter(Add1InPlace(Datatype::UINT64));
 

tiledb/sm/filter/test/unit_webp_pipeline.cc

@@ -0,0 +1,110 @@
+/**
+ * @file unit_webp_pipeline.cc
+ *
+ * @section LICENSE
+ *
+ * The MIT License
+ *
+ * @copyright Copyright (c) 2024 TileDB, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @section DESCRIPTION
+ *
+ * This set of unit tests checks running the filter pipeline with the webp
+ * filter.
+ */
+
+#include <test/support/tdb_catch.h>
+#include "filter_test_support.h"
+#include "test/support/src/mem_helpers.h"
+#include "test/support/src/whitebox_helpers.h"
+#include "tiledb/sm/filter/webp_filter.h"
+#include "tiledb/sm/tile/tile.h"
+
+using namespace tiledb::sm;
+
+TEST_CASE("Filter: Round trip WebpFilter RGB data", "[filter][webp]") {
+  tiledb::sm::Config config;
+  auto tracker = tiledb::test::create_test_memory_tracker();
+
+  uint64_t height = 100;
+  uint64_t width = 100;
+  uint64_t row_stride = width * 3;
+  auto tile = make_shared<WriterTile>(
+      HERE(),
+      constants::format_version,
+      Datatype::UINT8,
+      sizeof(uint8_t),
+      height * row_stride,
+      tracker);
+
+  // Write full image in a single tile with chunking enabled.
+  std::vector<uint8_t> data{0, 125, 255};
+  std::vector<uint8_t> expected_result(height * row_stride, 0);
+  for (uint64_t i = 0; i < height * width; i++) {
+    // Write three values for each RGB pixel.
+    for (uint64_t j = 0; j < 3; j++) {
+      CHECK_NOTHROW(tile->write(&data[j], i * 3 + j, sizeof(uint8_t)));
+      expected_result[i * 3 + j] = data[j];
+    }
+  }
+  // For the write process 10 rows at a time using tile chunking.
+  // The row stride is 300 bytes, so the tile chunk size is 3000 bytes.
+  uint64_t extent_y = 10;
+  WhiteboxWriterTile::set_max_tile_chunk_size(extent_y * row_stride);
+
+  FilterPipeline pipeline;
+  ThreadPool tp(4);
+  float quality = 100.0f;
+  bool lossless = true;
+  // NOTE: The extent_y parameter must respect chunk size or risk OOB access.
+  // This sets WebpFilter::extents_ which is passed to WebP API during encoding.
+  // If we set this to `height`, webp will reach past the end of chunked data.
+  pipeline.add_filter(WebpFilter(
+      quality,
+      WebpInputFormat::WEBP_RGB,
+      lossless,
+      extent_y,
+      width * 3,
+      Datatype::UINT8));
+  bool use_chunking = true;
+  CHECK(
+      pipeline.run_forward(&dummy_stats, tile.get(), nullptr, &tp, use_chunking)
+          .ok());
+
+  // Check the original unfiltered data was removed.
+  CHECK(tile->size() == 0);
+  CHECK(tile->filtered_buffer().size() != 0);
+
+  // Read the full image back with chunking disabled.
+  // NOTE: For the read case, WebP decoding APIs don't require height and width.
+  // Instead, WebP takes references to these values during unfiltering and sets
+  // them to the correct values after decoding is finished.
+  WhiteboxWriterTile::set_max_tile_chunk_size(constants::max_tile_chunk_size);
+  auto unfiltered_tile =
+      create_tile_for_unfiltering(height * row_stride, tile, tracker);
+  run_reverse(config, tp, unfiltered_tile, pipeline);
+
+  for (uint64_t i = 0; i < height * row_stride; i++) {
+    uint8_t value;
+    CHECK_NOTHROW(unfiltered_tile.read(&value, i, sizeof(uint8_t)));
+    CHECK(value == expected_result[i]);
+  }
+}

tiledb/sm/filter/webp_filter.cc

@@ -381,7 +381,6 @@ void WebpFilter::set_extents(const std::vector<ByteVecValue>& extents) {
     throw StatusException(Status_FilterError(
         "Tile extents too large; Max size WebP image is 16383x16383 pixels"));
   }
-  WriterTile::set_max_tile_chunk_size(extents_.first * extents_.second);
 }
 
 template void WebpFilter::set_extents<uint8_t>(

tiledb/sm/tile/tile.cc

@@ -98,10 +98,6 @@ uint32_t WriterTile::compute_chunk_size(
   return static_cast<uint32_t>(chunk_size64);
 }
 
-void WriterTile::set_max_tile_chunk_size(uint64_t max_tile_chunk_size) {
-  max_tile_chunk_size_ = max_tile_chunk_size;
-}
-
 /* ****************************** */
 /*   CONSTRUCTORS & DESTRUCTORS   */
 /* ****************************** */