Skip to content

Avoid potential infinite loop for rdpUpdate. #193

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

Merged

Conversation

Nexarian
Copy link
Contributor

@Nexarian Nexarian commented May 17, 2021

Summary

When frame acknowledgement is enabled, such as with the prototype of
egfx, the driver is at risk of going into an infinite loop if for some
reason an ack is missed.

While this line could fix it from xrdp: mm->mod->mod_frame_ack(mm->mod, 0, INT_MAX);,
the driver needs to be stable by itself.

The rationale here is that this is akin to a network socket timeout.
After a certain wait, it gives up. Same here.

Also, frame acknowledgement is a quality of service feature, and is not
strictly necessary for the functionality of remote desktop. In the worst
case, bandwidth calcuations between client and server are temporarily
incorrect.

200 was selected after some empirical testing, it could be tweaked.

Testing

[184680.280] rdpDeferredUpdateCallback: reschedule rect_id 542 rect_id_ack 541, retries: 197
[184680.284] rdpDeferredUpdateCallback: reschedule rect_id 542 rect_id_ack 541, retries: 198
[184680.289] rdpDeferredUpdateCallback: reschedule rect_id 542 rect_id_ack 541, retries: 199
[184680.293] rdpDeferredUpdateCallback: reschedule rect_id 542 rect_id_ack 541, retries: 200
[184680.298] rdpDeferredUpdateCallback: reschedule rect_id 542 rect_id_ack 541, retries: 201
[184680.298] rdpScheduleDeferredUpdate: clientCon->retries is 201 and has exceeded timeout. Overriding rect_id_ack.

When frame acknowledgement is enabled, such as with the prototype of
egfx, the driver is at risk of going into an infinite loop if for some
reason an ack is missed.

While this line could fix it from xrdp: `mm->mod->mod_frame_ack(mm->mod, 0, INT_MAX);`,
the driver needs to be stable by itself.

The rationale here is that this is akin to a network socket timeout.
After a certain wait, it gives up. Same here.

Also, frame acknowledgement is a quality of service feature, and is not
strictly necessary for the functionality of remote desktop. In the worst
case, bandwidth calcuations between client and server are temporarily
incorrect.

200 was selected after some empirical testing, it could be tweaked.
@metalefty metalefty merged commit 9990e8e into neutrinolabs:devel May 26, 2021
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