intel / node-realsense

MIT License
24 stars 21 forks source link

[SLAM]involking getOccupancyMapUpdate in 'tracking' callback direct will crash. #108

Closed haoyunfeix closed 7 years ago

haoyunfeix commented 7 years ago

Donna:

FIXME: The crash point is req.Resolve() in AsyncCallback function. The request pointer seems valid, but Resolve member seems inaccessible. Wrap the code block with setTimeout will walk around this issue. We need to fix it, or change the interface format by event to get map update data.

let slam;
slamModule.createInstance().then((instance) => {
  let slamOptions = {
    occupancyMapResolution: mapResolution,
  };
  return slam.setInstanceOptions(slamOptions);
}).then(() => {
  console.log('Starting SLAM...');
  slam.on('tracking', (result) => {
    sendCameraPose(result);
    // Works well.
    setTimeout(() => {
      slam.getOccupancyMapUpdate().then((mapData) => {
        sendOccupancyMap(mapData);
      });
    }, 0);
    // segfault.
    /*
    slam.getOccupancyMapUpdate().then((mapData) => {
      sendOccupancyMap(mapData);
    });
    */
    slam.getTrackingResult().then((result) => {
      updateAndSendFps(result);
      sendFishEyeFrame(result.frameData);
    });
  });
  return slam.start();
}).catch((error) => {
  console.log('error: ' + error);
});

Ningxin:

In current implementation, the GetOccupancyMapUpdate is invoked in the worker thread, however, SLAM MW is running on another dedicated thread where SlamEventHandler::module_output_ready is invoked. I highly suspect whether SLAM MW supports this multiple threads usage. It might cause crash when SLAM MW and worker thread access the occupancy map at simultaneously.

Are we able to make sure all SLAM APIs are invoked in the SLAM MW thread? The map data can be fetched in the SlamEventHandler::module_output_ready and notify JS with a copy of map data.

@DonnaWuDongxia , WDYT?

DonnaWuDongxia commented 7 years ago

@haoyunfeix I think this crash bug has been fixed by replacing the async mechanism. Would you please take a test and close it? Thank you very much!

DonnaWuDongxia commented 7 years ago

I think this bug could be closed. @haoyunfeix still can't find Ningxin in this repo.

DonnaWuDongxia commented 7 years ago

I will close it. Please reopen it if you think needed.