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

Refactor dlq queueing and locking #496

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from
Draft

Conversation

bjpetit
Copy link
Contributor

@bjpetit bjpetit commented Oct 30, 2023

I was digging into the dlq queueing code as I was trying to understand why I was getting these messages.
Received frame queue is out of control. Length
Upon some further digging, I believe the number of nodes my bpq instance registers may be triggering this message from time to time. That's why you'll see I bumped the message conditional to 15 in the change.
While I was digging in, I ended up doing a little refactoring on the queueing and locking in this area. The key changes are;

  • Eliminate the wake_up_mutex and just use the dlq_mutex.
  • Add a queue tail and eliminate walking the queue for each append

I'm currently testing this against my bpq station on one HF FX.25 port, one HF IL2P port and one VHF FX.25 port, and so far things are holding together ok.
One caveat: I haven't tried compiling or testing this against Windows as I don't have a windows system set up at the moment.
But leaving this PR here, in case it is interesting.

@bjpetit bjpetit force-pushed the queue-update branch 3 times, most recently from 0338e90 to c944372 Compare November 1, 2023 02:03
@wb2osz
Copy link
Owner

wb2osz commented Nov 6, 2023

Thank you for your suggestions to help clean up this mess.
It was a long time ago, but I recall that it took some effort to get it working correctly on both Linux and Windows.
As I recall, there was a difference in the behavior of semaphores. One one platform, they maintained a count, on the other they did not. My work around was to add another variable to indicate whether the queue reading thread was waiting on an empty queue. This might have something to do with two different mutexes, when you might expect one for all of the variables related to the queue maintenance.
I need some time to study this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants