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

Pin Monitor 100% CPU #22

Open
splitice opened this issue Apr 28, 2017 · 23 comments
Open

Pin Monitor 100% CPU #22

splitice opened this issue Apr 28, 2017 · 23 comments
Labels

Comments

@splitice
Copy link

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
Copy link
Owner

jcurl commented Apr 28, 2017

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
Copy link
Author

splitice commented Apr 28, 2017

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

@jcurl
Copy link
Owner

jcurl commented Apr 28, 2017

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
Copy link
Author

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
Copy link
Owner

jcurl commented Apr 28, 2017

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
Copy link
Author

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
Copy link
Author

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

jcurl added a commit that referenced this issue Apr 28, 2017
For some serial devices, such as ttyACM* USB devices, the ioctl(TIOCMIWAIT) returns an error. Now if an error occurs, we propogate it to the .NET implementation which already correctly ends the monitoring thread.

Previously, the error was ignored in a loop resulting in 100% CPU utilization.

Issue: DOTNET-85, #22
@jcurl
Copy link
Owner

jcurl commented Apr 28, 2017

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
Copy link
Owner

jcurl commented May 2, 2017

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

@splitice
Copy link
Author

splitice commented May 2, 2017

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
Copy link
Owner

jcurl commented May 2, 2017

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

@splitice
Copy link
Author

splitice commented May 2, 2017

looks fine to me

@jcurl
Copy link
Owner

jcurl commented May 2, 2017

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 added a commit that referenced this issue May 6, 2017
For some serial devices, such as ttyACM* USB devices, the ioctl(TIOCMIWAIT) returns an error. Now if an error occurs, we propogate it to the .NET implementation which already correctly ends the monitoring thread.

Previously, the error was ignored in a loop resulting in 100% CPU utilization.

Issue: DOTNET-85, #22
@jcurl
Copy link
Owner

jcurl commented May 6, 2017

Bugs fixed and now pushed to v2.x

@jcurl jcurl closed this as completed May 6, 2017
@splitice
Copy link
Author

@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
Copy link
Author

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

@jcurl
Copy link
Owner

jcurl commented Jul 31, 2017

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

@jcurl jcurl reopened this Jul 31, 2017
@splitice
Copy link
Author

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.

@jcurl jcurl added the bug label Apr 25, 2018
@pigwing
Copy link

pigwing commented Jul 7, 2018

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

@pigwing
Copy link

pigwing commented Jul 7, 2018

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
Copy link
Owner

jcurl commented Jul 7, 2018

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

@pigwing
Copy link

pigwing commented Jul 9, 2018

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

@pigwing
Copy link

pigwing commented Jul 9, 2018

i found that 2.1.2 always will let cpu 100%

@jcurl jcurl mentioned this issue Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants