Error objects become invalid and cause a crash (test-case supplied)

[Rush]

Took me a while to nail it down to this simple test case.

Couple of things:

• Requirement to trigger the bug: storage pool that has several volumes. Even fake ones may suffice.
• Combination of querying for existing and non existing storage volumes, and only in parallel, causes the crash.

'use strict';

var virt = require('libvirt');
let hypervisor = new virt.Hypervisor('qemu:///system');

if(!process.env.TEST_POOL_NAME) {
  console.log('Provide a TEST_POOL_NAME environment variable, to trigger a crash it needs to have a couple of volumes');
  process.exit(1);
}

hypervisor.connectAsync(() => {
    return hypervisor.lookupStoragePoolByNameAsync(process.env.TEST_POOL_NAME).then(pool => {
      pool.getVolumes((err, volumes) => {
        volumes.forEach((volumeName, idx) => {
          // below line intentionally queries missing storage volumes every 2 times
          let volumeNameToQuery = volumeName + ((idx % 2) ? 'INTENTIONAL_BREAK' : '');
          return pool.lookupStorageVolumeByNameAsync(volumeNameToQuery).then(volume => {
           return volume.getInfoAsync();
          });
        })
      });
    });
});

Stacktrace:

#0  0x00007ffff6bf228a in strlen () from /lib64/libc.so.6
#1  0x00000000008eb435 in v8::String::NewFromUtf8(v8::Isolate*, char const*, v8::NewStringType, int) ()
#2  0x00007ffff48f9ebe in Nan::imp::Factory<v8::String>::New (value=0x0, length=-1) at ../../nan/nan_implementation_12_inl.h:265
#3  0x00007ffff48fd881 in Nan::New<v8::String, char const*> (arg0=0x0) at ../../nan/nan_new.h:208
#4  0x00007ffff48fa007 in Nan::New (value=0x0) at ../../nan/nan_new.h:313
#5  0x00007ffff49111a2 in NLV::Error::Getter (property=..., info=...) at ../src/error.cc:180
#6  0x00007ffff490a218 in Nan::imp::GetterCallbackWrapper (property=..., info=...) at ../../nan/nan_callbacks_12_inl.h:190
#7  0x0000000000904210 in v8::internal::PropertyCallbackArguments::Call(void (*)(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&), v8::Local<v8::Name>) ()
#8  0x0000000000bef85d in v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*, v8::internal::LanguageMode) ()
#9  0x0000000000c04d23 in v8::internal::Object::GetProperty(v8::internal::LookupIterator*, v8::internal::LanguageMode) ()
#10 0x0000000000c048c3 in v8::internal::Object::GetPropertyOrElement(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::LanguageMode) ()
#11 0x0000000000ca4759 in v8::internal::BasicJsonStringifier::Result v8::internal::BasicJsonStringifier::Serialize_<false>(v8::internal::Handle<v8::internal::Object>, bool, v8::internal::Handle<v8::internal::Object>) ()
#12 0x0000000000ca5248 in v8::internal::Runtime_BasicJSONStringify(int, v8::internal::Object**, v8::internal::Isolate*) ()

A trivial fix for src/error.cc:180 to account for error_->message being NULL:

    return info.GetReturnValue().Set(Nan::New(error_->message ? error_->message : "(NULL)").ToLocalChecked());

works around the issue but I'd rather seek a real solution. @mbroadst - any ideas?

[Rush]

Uh, just noticed that is has been fixed by 46ce09e

Can we have a new release?

[c4milo]

I will publish a new release shortly.

[c4milo]

v1.1.3 is up in NPM @Rush.

[Rush]

@c4milo Thank you for quick action!

[c4milo]

No problem at all @Rush. Thanks for reporting this.

[mbroadst]

@Rush @c4milo sorry I was away on vacation, glad you worked it out!

[Rush]

@mbroadst I hope you had a great vacation. You guys @mbroadst @c4milo rock. :) Happy Easter.