Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lingering device with sweepjs bindings #69

Open
dcyoung opened this issue Apr 13, 2017 · 4 comments
Open

lingering device with sweepjs bindings #69

dcyoung opened this issue Apr 13, 2017 · 4 comments

Comments

@dcyoung
Copy link

dcyoung commented Apr 13, 2017

Something appears to be persisting after using the sweepjs bindings. Attempting to use an alternate method of interfacing with the device (ie: the sweep-visualizer) after running the sweepjs example requires that the device be reset. This is not the case with the sweeppy python bindings. My best guess is that the python bindings properly cleanup and and destroy the device on exit, while perhaps the sweepjs bindings do not?

@daniel-j-h
Copy link
Collaborator

The Sweep device is always getting cleaned up by means of a ref counted ptr

  • // Ref-counted to keep alive until last in-flight callback is finished
    std::shared_ptr<::sweep_device> device;
  • Sweep::Sweep(const char* port) {
    auto devptr = ::sweep_device_construct_simple(port, ErrorToException{});
    auto defer = [](::sweep_device_s dev) { ::sweep_device_destruct(dev); };
    device = {devptr, defer};
    }

Ref counted because there might be a async. callback still in flight.

@dcyoung
Copy link
Author

dcyoung commented Apr 26, 2017

Hmmm.... Those links make sense to me, but #71 did not appear to fix the issue.

Upon further inspection, it seems the example index.js was not terminating. Instead it was hung up somehow. This was preventing other applications from using the device.

First, I noticed that index.js does not explicitly call stopScanning. This shouldn't be an issue, as the destructor from libsweep should stop scanning, but it should be included for reference anyhow.

I was seeing odd behavior when experimenting with index.js. If I placed a stopScanning() call after the sweep.scan(), it appeared to me that the callback passed in the sweep.scan() method was never being called, and the program hung indefinitely. With limited experimentation, I was able to get the example to print the samples, stopScanning, and terminate properly. a3f9553

But I had to place the stopScanning() call inside the callback. See

sweep.stopScanning();

Obviously this isn't ideal. @daniel-j-h Do you have any ideas what might be causing this behavior.

@daniel-j-h
Copy link
Collaborator

I was seeing odd behavior when experimenting with index.js. If I placed a stopScanning() call after the sweep.scan(), it appeared to me that the callback passed in the sweep.scan() method was never being called, and the program hung indefinitely.

The problem here is the sweep.scan function is an async. function, which means it takes a callback from the user which it will call on completion - the function itself returns immediately. For example:

a();

sweep.scan((err, result) => {
  b();
});

c();

The only guarantee you have is: a() will be called before b() and c(). You have no guarantees wrt. b() and c()'s ordering. sweep.scan enqueues a async. worker for libuv (because vanilla v8 is single threaded) which will call the callback on completion.

@dcyoung
Copy link
Author

dcyoung commented May 1, 2017

I responded in the PR #75
Agree that this is not a proper fix, but the example is broken currently. Any ideas what is causing the behavior or what a proper fix might be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants