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

SerialPort.list() crashes Node when used with brother device #2259

Closed
rb-serin opened this issue Jun 3, 2021 · 11 comments
Closed

SerialPort.list() crashes Node when used with brother device #2259

rb-serin opened this issue Jun 3, 2021 · 11 comments

Comments

@rb-serin
Copy link

rb-serin commented Jun 3, 2021

SerialPort.list() crashes Node ( used on electron) when a Brother printer is attached

(Please answer all 3)

  • What are you trying to do?
    Listing all devices attached to the machine

  • What happens?
    Node crashes leaving a blank page on electron renderer process

  • What should have happened?
    Getting a list of devices or an exception

Code to Reproduce the Issue

const SerialPort = require('serialport')
SerialPort.list()

Versions, Operating System and Hardware

  • SerialPort@ 7.x and 9.x
  • Node.js v14.15.4
  • Windows? Linux? Mac? Windows
  • Hardware and chipset? (Prolific/FTDI/Other) Intel core i5 with Brother printer attached

This is the same problem as #1518
See also https://www.gitmemory.com/issue/serialport/node-serialport/2155/771687008

@GazHank
Copy link
Contributor

GazHank commented Jun 7, 2021

Hi @rb-serin

Would you be able to confirm the specific version(s) of electron and serial port that you are using?

Also, could you confirm if you are facing the issue of the device not being assigned a COM port per the prior issue you referenced?

If the COM port isn't being assigned by windows you might want to manually configure the COM port within device manager (there are some instructions here: https://www.virtual-serial-port.org/articles/com-port-on-windows-10/#assign ). Although even if that does resolve your issue it sounds like the error handling/ fault tolerance could be improved.

@GazHank
Copy link
Contributor

GazHank commented Jul 5, 2021

This might be related to #2155 as both seem to relate to unexpected behaviour; in both cases I think we need more info to be able to investigate and debug

@GazHank GazHank added needs-info Additional information required and removed needs-testing labels Jul 5, 2021
@nyholmjan
Copy link

I can reliably reproduce this issue. The key part is to query for ports while windows is installing usb to serial drivers. It can be reproduced simply by removing the usb to serial drivers of the device from device manager and then unplugging the device and plugging it in again, the crash will happen when serialport.list() is called while the drivers are being reinstalled.

Versions, Operating System and Hardware

  • serialport: 9.2.0
  • electron: 13.2.1
  • os: windows 10
  • hardware: u-blox EVK-M101

Code to reproduce

const serialport = require("serialport");
window.setInterval(() => {
    serialport.list().then(ports => {
        console.log(ports)
}, 1000);

@GazHank
Copy link
Contributor

GazHank commented Sep 6, 2021

I wonder if this is due to changes being made to the data stored in the windows registry as we are executing the list(). Since much of the data returned by the list operation comes from the registry if it's content changes mid process it could causes this breakage. I can't see any unhandled exceptions based on a quick scan of the code, but that does feel like the most likely issue.

I had previously thought about avoiding direct registry access, and using the windows APIs more fully to provide us a little more abstraction, in the hope that the APIs would provide better fault tollerance and handling of weird edge cases. I'll try to take a look at this again when I get some time. If anyone wants to take a look at the code in question and perform some debugging, the EIO_List and associated methods it calls can be found @ https://github.com/serialport/node-serialport/blob/master/packages/bindings/src/serialport_win.cpp#L782

@GazHank
Copy link
Contributor

GazHank commented Sep 28, 2021

thanks @pseudorandomseed that's great news!

I mean that you have access to a device that exhibits this issues... I'm really sorry that this is causing crashes for you (and your customers).

Would you be able to try adding print statements to the EIO_List method per @reconbot suggestion: #2155 (comment)

This would allow us to see at which step of the process the crash is occurring? (let me know if you would like any help or support to make the changes, or to setup your windows machine to be able to build the code)

@reconbot
Copy link
Member

That's about right

@caffeinatedbits
Copy link
Contributor

@reconbot see #2325

@reconbot
Copy link
Member

Can someone test serialport@10.0.0 to make sure this is still good?

@nyholmjan
Copy link

Can someone test serialport@10.0.0 to make sure this is still good?

It hasn't been verified that the similar issue with the u-blox devices described above has the same cause, but I have tested that one with serialport@10.0.0 and the issue still persists.

@GazHank
Copy link
Contributor

GazHank commented Dec 16, 2021

Can someone test serialport@10.0.0 to make sure this is still good?

It hasn't been verified that the similar issue with the u-blox devices described above has the same cause, but I have tested that one with serialport@10.0.0 and the issue still persists.

Are you also experiencing the u-blox device issue with the latest v9 release too? If so I wonder if we have a different defect we need to address for that device

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week no further activity occurs. Feel free continue the discussion or ask for a never-stale label to keep it open. If this is a support issue, consider sharing it on stack overflow to get more eyes on your problem.

@stale stale bot added the stale-issue label Apr 16, 2022
@stale stale bot closed this as completed Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants