Parent objects get garbage collected before children' async operations finish which can lead to a crash

[Rush]

Consider this simplistic case case that can cause Segmentation fault:

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

function createStorageVolume(xml) {
  hypervisor.lookupStoragePoolByNameAsync('some-pool').then(function(pool) {
     // at this point of time pool is referenced for the last time,
     // after createVolumeAsync starts, the garbage collector is
     // free to free the native object any time
     return pool.createVolumeAsync(xml);
  });
}

Solution that fixes it is:

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

function createStorageVolume(xml) {
  hypervisor.lookupStoragePoolByNameAsync('some-pool').then(function(pool) {
     return pool.createVolumeAsync(xml).then(function(volume) {
        var _pool = pool; // keep reference of pool until the method finishes
        return volume;
     });;
  });
}

I am yet to devise a pure node-libvirt test case. Anyway, a way to test it is to try to create many volumes in parallel using the above pattern and preferably call node with node --expose-gc, furthermore add code if(global.gc) setInterval(global.gc, 1); which should make sure the GC kicks in.

I think it may be somehow possible to increase the refcount in node-libvirt itself but it's hard for me to wrap my head around all those wrapper classes.

[mbroadst]

@Rush yeah, hmm memory is pretty tricky in these cases. There are a few ways to ensure the objects stick around (I think the persistent handle, as well as an explicit internal counter that can be increased), but then you need to ensure it's dereffed when it's no longer needed - big headaches here.

Would it be possible to write a simple "manual" test case that we could run as part of the automated test suite? We could run that one with expose-gc so you could explicitly call for garbage collection in it, and would serve as a great starting point for finding a solution to the problem

[mbroadst]

@Rush I'm running the following snippit:

'use strict';
var virt = require('..'),
    fixture = require('./lib/helper').fixture;

// continuously run gc every 100ms
if (global.gc) setInterval(global.gc, 100);

var xml = fixture('storage_volume.xml');
var hv = new virt.Hypervisor('test:///default');
hv.connectAsync()
  .then(function() { return hv.lookupStoragePoolByNameAsync('default-pool'); })
  .then(function(pool) { return pool.createVolumeAsync(xml); })
  .catch(function(err) { console.log('error: ', err); });

and getting this output:

mbroadst@retinoid:node-libvirt (master=)$ node --expose-gc test/issue65.js
Assertion failed: (result == 0), function ClearHandle, file ../src/nlv_object.h, line 60.
Abort trap: 6

does that look like what you're running into?

[mbroadst]

@Rush update here, that does look like the relevant error however I think we've tapped out my creativity on solving the problem 😄 I've spend about an hour trying to figure out how to "attach" the parent to the child object from the C++ side, but it seems incredibly difficult.

I did, on the other hand, find a solution through javascript which might be the way to go (but then we've got a bit of work ahead of us) through this monkey-patch in index.js:

var $createVolumeAsync = libvirt.StoragePool.prototype.createVolumeAsync;
libvirt.StoragePool.prototype.createVolumeAsync = function(options) {
  var self = this;
  return $createVolumeAsync.call(self, options)
    .then(function(volume) {
      volume._pool = self;
      return volume;
    });
};