EttusResearch / uhd

The USRP™ Hardware Driver Repository
http://uhd.ettus.com
Other
974 stars 654 forks source link

host: Fix RFNoC graph action queue lockup on action exceptions #730

Closed hannodewind closed 5 months ago

hannodewind commented 7 months ago

Pull Request Details

Description

Processing of the action queue gets locked up when any action being executed in the send_action call throws an exception. Exceptions are not caught in the loop handling the action queue, resulting in the handling_ongoing queue locking flag to never be released. Any subsequent call to enqueue_action will return on the early exit with the assumption that we're already handling the actions, yet the previous handler exited with an exception.

This fix uses a RAII wrapper rather than a manually claimed and released atomic flag to ensure that the handling_ongoing will be released even under exceptional conditions.

Related Issue

Relates to issue #611

Which devices/areas does this affect?

UHD hosts using RFNoC graph

Testing Done

X310 with dual 10GbE links to server, running both RF inputs at 200MHz sample rate using 2x RX streamers. Stress the server with CPU load (can use stress-ng), inducing UDP packet drops. (Also relates to #611, which stressed the link using iperf, probably also causing UDP packet drops). At some point (difficult to reproduce, but does happen every so often), one of the RX streamers will experience an overrun, which calls the _overrun_handler -> ACTION_KEY_RX_RESTART_REQ which calls get_time_now(), doing a peek64 to the device. This peek64 then throws an exception due to an ACK timeout.

This exception is caught all the way up in thread that called recv on the RX streamer, but the stream is irrecoverable since the graph action queue is locked up.

Checklist

github-actions[bot] commented 7 months ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

hannodewind commented 7 months ago

I have read the CLA Document and I hereby sign the CLA

hannodewind commented 7 months ago

recheck

hannodewind commented 7 months ago

recheck

mbr0wn commented 7 months ago

@hannodewind Don't worry about the CLA checker bot, it's a misconfig on our end (I think).

This is all we need for now:

image

mbr0wn commented 5 months ago

@hannodewind I think I will modify this to use UHD's scope_exit instead of your bespoke class, but otherwise, this is a fantastic change. Kudos to figuring out the issue, and for modifying in such a great way so far inside its guts!

hannodewind commented 5 months ago

@mbr0wn Thank you for the feedback, I am keen to see the scope_exit implementation. Let me know if there is anything else I can/should do on this PR, always happy to assist!

joergho commented 5 months ago

The change is now in master: 0f2007fc9