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

app_rpt: Correct crash when using rpt page #377

Merged
merged 2 commits into from
Sep 14, 2024
Merged

app_rpt: Correct crash when using rpt page #377

merged 2 commits into from
Sep 14, 2024

Conversation

KB4MDD
Copy link
Collaborator

@KB4MDD KB4MDD commented Jul 31, 2024

This updates app_rpt to address a crash that occurred when using the rpt page command.

ast_sendtext should not require locking according to the documentation, but that does not seem to be the case. In some instances, the main processing loop had control of the channel. ast_sendtext would cause an assert abort in channel.c.

This was shown by the message ast_sendtext_data: Thread LWP 88692 is blocking 'Radio/usb_2231', already blocked by thread LWP 88688 in procedure ast_waitfor_nandfds.

Adding rpt_mutex_lock around ast_sendtext resolved the problem.

Corrected an if statement in the same routine that was already flagged in the code as a bug.

Remove chan_usbradio as a destination for paging. chan_usbradio does not implement paging. Sending pages to chan_usbradio resulted in a stuck PTT.

Applied coding standards to the routine.

This closes #375 and #371.

This updates app_rpt to address a crash that occurred when using the rpt page command.

ast_sendtext should not require locking according to the documentation, but that does
not seem to be the case.  In some instances, the main processing loop had control of
the channel.  ast_sendtext would cause an assert abort in channel.c.

This was shown by the message ast_sendtext_data: Thread LWP 88692 is blocking 'Radio/usb_2231',
already blocked by thread LWP 88688 in procedure ast_waitfor_nandfds.

Adding rpt_mutex_lock around ast_sendtext resolved the problem.

Corrected an if statement in the same routine that was already flagged in the code
as a bug.

Applied coding standards to the routine.

This closes #375.
@KB4MDD KB4MDD added the bug Something isn't working label Jul 31, 2024
@KB4MDD KB4MDD self-assigned this Jul 31, 2024
This updates app_rpt to address a crash that occurred when using the rpt page command.

ast_sendtext should not require locking according to the documentation, but that does not seem to be the case. In some instances, the main processing loop had control of the channel. ast_sendtext would cause an assert abort in channel.c.

This was shown by the message ast_sendtext_data: Thread LWP 88692 is blocking 'Radio/usb_2231', already blocked by thread LWP 88688 in procedure ast_waitfor_nandfds.

Adding rpt_mutex_lock around ast_sendtext resolved the problem.

Corrected an if statement in the same routine that was already flagged in the code as a bug.

Remove chan_usbradio as a destination for paging.  chan_usbradio does not implement paging. Sending pages to chan_usbradio resulted in a stuck PTT.

Applied coding standards to the routine.

This closes #375 and #371.
Copy link
Collaborator

@Allan-N Allan-N left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good.

@Allan-N
Copy link
Collaborator

Allan-N commented Aug 12, 2024

On 08/10 (on slack), I asked @InterLinked1 about this change and the usage of &myrpt->blocklock. He replied with :

I had taken a look at work earlier this week but forgot to respond, the PR looks good to me. Asterisk does recursive locking (not normally enabled by default in pthreads) so the same thread can acquire a lock multiple times. I don't really like that idea personally but it's commonplace in Asterisk so a perfectly okay thing to do here.
blocklock was a lock I added specifically for channel servicing like is the case here so that makes sense to me. It sounds like Danny tested the change and found it resolved and having that feedback is always good too.

@Allan-N Allan-N merged commit dcfe5d6 into master Sep 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

app_rpt crashes occasionally when sending POCSAG pages to chan_simpleusb
3 participants