Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#9922 closed Patch - Bug Fix (fixed)

Frontend network control sends two responses after 'play program'

Reported by: Gregory Moyer <moyerg@…> Owned by: Raymond Wagner
Priority: minor Milestone: 0.25
Component: MythTV - General Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

My patch for #9725 overlooked the fact that the 'play program' command was relying on empty responses not being sent to the client.

In the processPlay method in networkcontrol.cpp for a 'play program' command, the result string is cleared when the NETWORK_CONTROL event to actually trigger playback is sent. Before the patch, when this function returned with an empty result, no message would have been sent back to the client and the client would still be waiting at this point. Once the playback request is received, networkcontrol.cpp will see a NETWORK_CONTROL RESPONSE event in the function customEvent and that will trigger the "OK" response to the client. This is the only place I found in which the result is cleared while a response is expected via an event later.

Now, after the fix from #9725, the empty response sends a prompt back to the user immediately and a short time later when the RESPONSE event is received, a second message of "OK" with another prompt is sent to the user. Therefore, in a netcat session, the user seems something like the attached nc-broken.log output.

My approach to correct this involves setting the response to a null QString (as QString defines null using the no-arg constructor) rather than an empty string when no response should be sent. Instead of checking for an empty string, the if check that was removed in #9725 is now verifying that the response is not null. This allows for both scenarios where an empty message can be returned to the user (such as when a 'query recordings' command is issued and no recordings exist), but an immediate response can also be stopped to allow for a delayed response triggered via other means, such as an event in the case of a 'play program' request.

This patch also changes the default response to 'query recordings' to a an empty string instead of a null string to comply with the above.

Attachments (4)

fenc-stop-sending-multiple-responses.patch (1.7 KB) - added by Gregory Moyer <moyerg@…> 8 years ago.
mythfrontend.version.log (729 bytes) - added by Gregory Moyer <moyerg@…> 8 years ago.
nc-broken.log (156 bytes) - added by Gregory Moyer <moyerg@…> 8 years ago.
nc-fixed.log (153 bytes) - added by Gregory Moyer <moyerg@…> 8 years ago.

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by Gregory Moyer <moyerg@…>

Changed 8 years ago by Gregory Moyer <moyerg@…>

Attachment: mythfrontend.version.log added

Changed 8 years ago by Gregory Moyer <moyerg@…>

Attachment: nc-broken.log added

Changed 8 years ago by Gregory Moyer <moyerg@…>

Attachment: nc-fixed.log added

comment:1 Changed 8 years ago by Raymond Wagner

Owner: set to Raymond Wagner
Status: newaccepted

Thank you for tracking this down and producing another patch, but I'm going to go a different direction in fixing this one. Rather than add special handling for commands that respond asynchronously, I'm going to enforce everything operate synchronously, such that there will always be a response to send back to the client.

comment:2 Changed 8 years ago by Github

Milestone: unknown0.25
Resolution: fixed
Status: acceptedclosed

Make "play program" network control call synchronous.

This moves the response from "play program" while inside the playbackbox into the handling thread, rather than a separate asynchronous result. This may result in some concurrency issues as the entire network control server is locked until that thread to that client responds. Due to the already very coarse locking, it should not be significantly noticeable.

Fixes #9922

Branch: master Changeset: 731949d69f2d9c6e993f57cf60a277413a94872d

comment:3 Changed 8 years ago by Github

Adjust timeouts for frontend control socket

Increases timeout for startup of recording playback from 2 seconds to 10 seconds. Move all such timeouts to FE_SHORT_TO and FE_LONG_TO, of 2 and 10 seconds respectively.

Refs #9922

Branch: master Changeset: 691d1368f373327685b8ea24015afd85e9d65a1e

Note: See TracTickets for help on using tickets.