Improve thread-safety of error and warning message handling.

Ensures all message reading occurs before thread shutdown.

docs/src/content/docs/changelog/v0.35.0.md

@@ -20,7 +20,7 @@ slug: changelog/v0.35.0
 
 * Upgrade to libvips v8.18.0 for upstream bug fixes.
 
-* Ensure TIFF output `bitdepth` option is limited to 1, 2 or 4.
+* Improve thread-safety of error (and warning) messages.
 
 * Deprecate Windows 32-bit (win32-ia32) prebuilt binaries.
 

lib/index.d.ts

@@ -1479,8 +1479,8 @@ declare namespace sharp {
         xres?: number | undefined;
         /** Vertical resolution in pixels/mm (optional, default 1.0) */
         yres?: number | undefined;
-        /** Reduce bitdepth to 1, 2 or 4 bit (optional) */
+        /** Reduce bitdepth to 1, 2 or 4 bit (optional, default 8) */
-        bitdepth?: 1 | 2 | 4 | undefined;
+        bitdepth?: 1 | 2 | 4 | 8 | undefined;
         /** Write 1-bit images as miniswhite (optional, default false) */
         miniswhite?: boolean | undefined;
         /** Resolution unit options: inch, cm (optional, default 'inch') */

src/binding.gyp

@@ -21,7 +21,6 @@
         'defines': [
           '_VIPS_PUBLIC=__declspec(dllexport)',
           '_ALLOW_KEYWORD_MACROS',
-          '_HAS_EXCEPTIONS=1',
           'G_DISABLE_ASSERT',
           'G_DISABLE_CAST_CHECKS',
           'G_DISABLE_CHECKS'
@@ -149,8 +148,7 @@
           ['OS == "win"', {
             'defines': [
               '_ALLOW_KEYWORD_MACROS',
-              '_FILE_OFFSET_BITS=64',
+              '_FILE_OFFSET_BITS=64'
-              '_HAS_EXCEPTIONS=1'
             ],
             'link_settings': {
               'libraries': [

src/metadata.cc

@@ -152,7 +152,12 @@ class MetadataWorker : public Napi::AsyncWorker {
       // PNG comments
       vips_image_map(image.get_image(), readPNGComment, &baton->comments);
     }
-
+    // Handle warnings
+    std::string warning = sharp::VipsWarningPop();
+    while (!warning.empty()) {
+      baton->warnings.push_back(warning);
+      warning = sharp::VipsWarningPop();
+    }
     // Clean up
     vips_error_clear();
     vips_thread_shutdown();
@@ -162,13 +167,9 @@ class MetadataWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto& warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
-    while (!warning.empty()) {
       debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-      warning = sharp::VipsWarningPop();
     }
-
     if (baton->err.empty()) {
       Napi::Object info = Napi::Object::New(env);
       info.Set("format", baton->format);

src/metadata.h

@@ -57,6 +57,7 @@ struct MetadataBaton {
   size_t gainMapLength;
   MetadataComments comments;
   std::string err;
+  std::vector<std::string> warnings;
 
   MetadataBaton():
     input(nullptr),

src/pipeline.cc

@@ -1280,12 +1280,21 @@ class PipelineWorker : public Napi::AsyncWorker {
       } else {
         if (baton->input->failOn == VIPS_FAIL_ON_WARNING) {
           (baton->err).append("Warning treated as error due to failOn setting");
-          baton->errUseWarning = true;
         } else {
           (baton->err).append("Unknown error");
         }
       }
     }
+    // Handle warnings
+    std::string warning = sharp::VipsWarningPop();
+    while (!warning.empty()) {
+      if (baton->input->failOn == VIPS_FAIL_ON_WARNING) {
+        (baton->err).append("\n").append(warning);
+      } else {
+        (baton->warnings).push_back(warning);
+      }
+      warning = sharp::VipsWarningPop();
+    }
     // Clean up libvips' per-request data and threads
     vips_error_clear();
     vips_thread_shutdown();
@@ -1295,15 +1304,8 @@ class PipelineWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto &warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
+      debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-    while (!warning.empty()) {
-      if (baton->errUseWarning) {
-        (baton->err).append("\n").append(warning);
-      } else {
-        debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-      }
-      warning = sharp::VipsWarningPop();
     }
     if (baton->err.empty()) {
       int width = baton->width;

src/pipeline.h

@@ -204,6 +204,7 @@ struct PipelineBaton {
   bool jxlLossless;
   VipsBandFormat rawDepth;
   std::string err;
+  std::vector<std::string> warnings;
   bool errUseWarning;
   int keepMetadata;
   int withMetadataOrientation;

src/stats.cc

@@ -96,7 +96,12 @@ class StatsWorker : public Napi::AsyncWorker {
         (baton->err).append(err.what());
       }
     }
-
+    // Handle warnings
+    std::string warning = sharp::VipsWarningPop();
+    while (!warning.empty()) {
+      baton->warnings.push_back(warning);
+      warning = sharp::VipsWarningPop();
+    }
     // Clean up
     vips_error_clear();
     vips_thread_shutdown();
@@ -106,11 +111,8 @@ class StatsWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto& warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
-    while (!warning.empty()) {
       debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-      warning = sharp::VipsWarningPop();
     }
     if (baton->err.empty()) {
       // Stats Object

src/stats.h

@@ -45,6 +45,7 @@ struct StatsBaton {
   int dominantBlue;
 
   std::string err;
+  std::vector<std::string> warnings;
 
   StatsBaton():
     input(nullptr),

test/unit/metadata.js

@@ -642,7 +642,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile', async () => {
-    const data = await sharp(fixtures.inputJpgWithExif)
+    const data = await sharp(fixtures.inputJpgWithExif, { failOn: 'error' })
       .keepIccProfile()
       .toBuffer();
 
@@ -675,7 +675,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile, avoid colour transform', async () => {
-    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile)
+    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile, { failOn: 'error' })
       .keepIccProfile()
       .raw()
       .toBuffer();
@@ -721,7 +721,7 @@ describe('Image metadata', () => {
   });
 
   it('transform to invalid ICC profile emits warning', async () => {
-    const img = sharp({ create })
+    const img = sharp({ create, failOn: 'error' })
       .png()
       .withIccProfile(fixtures.path('invalid-illuminant.icc'));