RJCP.DLL.SerialPortStream icon indicating copy to clipboard operation
RJCP.DLL.SerialPortStream copied to clipboard

Pin Monitor 100% CPU

Open splitice opened this issue 8 years ago • 23 comments

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.

splitice avatar Apr 28 '17 03:04 splitice

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?

jcurl avatar Apr 28 '17 08:04 jcurl

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

splitice avatar Apr 28 '17 08:04 splitice

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).

jcurl avatar Apr 28 '17 12:04 jcurl

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.

splitice avatar Apr 28 '17 12:04 splitice

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).

jcurl avatar Apr 28 '17 13:04 jcurl

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 avatar Apr 28 '17 13:04 splitice

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

splitice avatar Apr 28 '17 13:04 splitice

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 avatar Apr 28 '17 14:04 jcurl

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

jcurl avatar May 02 '17 06:05 jcurl

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,

splitice avatar May 02 '17 06:05 splitice

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

jcurl avatar May 02 '17 06:05 jcurl

looks fine to me

splitice avatar May 02 '17 09:05 splitice

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 avatar May 02 '17 19:05 jcurl

Bugs fixed and now pushed to v2.x

jcurl avatar May 06 '17 14:05 jcurl

@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 avatar Jul 31 '17 07:07 splitice

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

splitice avatar Jul 31 '17 07:07 splitice

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

jcurl avatar Jul 31 '17 10:07 jcurl

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.

splitice avatar Jul 31 '17 11:07 splitice

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

pigwing avatar Jul 07 '18 01:07 pigwing

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

pigwing avatar Jul 07 '18 14:07 pigwing

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

jcurl avatar Jul 07 '18 19:07 jcurl

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

pigwing avatar Jul 09 '18 09:07 pigwing

i found that 2.1.2 always will let cpu 100%

pigwing avatar Jul 09 '18 14:07 pigwing