jcurl / RJCP.DLL.SerialPortStream

SerialPortStream is an independent implementation of System.IO.Ports.SerialPort and SerialStream for better reliability and maintainability. Default branch is 2.x and now has support for Mono with help of a C library.
Microsoft Public License
628 stars 197 forks source link

Pin Monitor 100% CPU #22

Open splitice opened 7 years ago

splitice commented 7 years ago

The TIOCMIWAIT ioctl returns EINVAL on some drivers/hardware. This results in 100% CPU usage for the thread as instead of blocking the function call immediately returns.

Perhaps it would be better to just introduce a delay (sleep) in cases like this since you can't wait on pin changes on this hardware.

jcurl commented 7 years ago

Thanks for the bug report! I'd also like to document this in the wiki. What hardware specifically do you have that shows this behaviour?

splitice commented 7 years ago

It's the g_serial driver for a USB virtual serialport (/dev/ttyACM0) interfacing with a TI CC2531.

jcurl commented 7 years ago

Investigated the issue. The proper solution is to propogate the error to UnixNativeSerial class which will abort all pin monitoring operations. So instead of polling in case of this error, I just stop. I assume that if it doesn't work once, it will stop working. I don't expect that the ioctl() should return an error otherwise (except for EAGAIN or EWOULDBLOCK, in which case we'd try again immediately).

splitice commented 7 years ago

I think that it's a fair bet for EINVAL, perhaps restrict it to just that error code for now (unless a case of a different code comes up).

For our information if this functionality is (Pin Monitoring) is disabled, what effect does it have on functionality? Might be good to document too.

jcurl commented 7 years ago

I'll continue the loop for only EAGAIN or EWOULDBLOCK as a signal may occur at any time. All others I'll treat as fatal. When the monitoring thread stops, a user will no longer get notification of pin state changes CTS, DTR, DCD and RI.

I had an old phone that also provided a /dev/ttyACM0 interface, unfortunately, I can't test with that (the ioctl works). I've tested by changing code in such a way that the error occurs and I get the result I expect (no 100% CPU, the thread stops and the program continues as expected).

splitice commented 7 years ago

Could I make a suggestion?

A better failure mode for devices that don't support waiting (and those who fail at it) would be to update pin changes using a thread with a fixed wait time (i.e 10ms) IMHO.

We don't use that functionality currently, so it's not an issue for us. Just saying.

splitice commented 7 years ago

And I'm happy to test any changes you make on Monday on the hardware, no issue.

jcurl commented 7 years ago

I've made a first draft in github with the branch "bugfix/modem". There are a few other fixes present when I reviewed the code. I'll look over your suggestion and see what I can do.

jcurl commented 7 years ago

Hu, wondering if you had the chance to test before I merge to the main branch.

splitice commented 7 years ago

Sorry I haven't my hardware got feature frozen for a demos.

Unfortunately that's the only one I have built at the moment. Another one will be getting built very soon, probably early next week,

jcurl commented 7 years ago

Would you be able to review the code? I'd like to be ready to merge on the weekend. Thanks for your help.

splitice commented 7 years ago

looks fine to me

jcurl commented 7 years ago

Just did a review of my own after a few days. I've found a few bugs, so I'll have to clean them up first.

jcurl commented 7 years ago

Bugs fixed and now pushed to v2.x

splitice commented 7 years ago

@jcurl I tested your newest version today. The issue is back.

strace shows:

[pid  2142] ioctl(3, TIOCMIWAIT, 0x1e0) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCGICOUNT, 0xb3afedd4) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMIWAIT, 0x1e0) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCGICOUNT, 0xb3afedd4) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMIWAIT, 0x1e0) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCGICOUNT, 0xb3afedd4) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMIWAIT, 0x1e0) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCGICOUNT, 0xb3afedd4) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMIWAIT, 0x1e0) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCGICOUNT, 0xb3afedd4) = -1 EINVAL (Invalid argument)
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
[pid  2142] ioctl(3, TIOCMGET^C, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS]) = 0
splitice commented 7 years ago

The problem appears to be that the pthread continues to get recreated after a failure.

jcurl commented 7 years ago

Just a sanity question, did you update and use the newer libnserial package? Did you compile from source or install the debian package?

splitice commented 7 years ago

yes using the latest libnserial compiled from git, and the latest .net side from NuGet.

I forgot to mention that that strace is above is grep'ed with ioctl.

I did some debugging and found that although the pthread is exited on failure, it gets restarted when requested again.

pigwing commented 6 years ago

hi, i have the same problem in raspberry pi 3 b+

pigwing commented 6 years ago

i found that,i am use 2.1.2 version the cpu not 100% and the 2.1.4 when i use it (serialportstream object singleton) read while 7 or 8 hours my raspberry cpu will cpu 100% all time.and that 2.1.2 will not

jcurl commented 6 years ago

I'm open for patches to review if you can provide a generic solution without regressions for standard hardware.

pigwing commented 6 years ago

i have catch crash info double free or corruption (fasttop): 0x5c011c18 ***

pigwing commented 6 years ago

i found that 2.1.2 always will let cpu 100%