Ignore screen size changes which don't change anything#203
Merged
Nexarian merged 3 commits intoneutrinolabs:develfrom Jul 30, 2022
Merged
Ignore screen size changes which don't change anything#203Nexarian merged 3 commits intoneutrinolabs:develfrom
Nexarian merged 3 commits intoneutrinolabs:develfrom
Conversation
matt335672
force-pushed
the
resize_to_same_size
branch
from
eca1d70 to
6ad2724
Compare
Member
Author
|
After some more testing with RFX, I've found a other places where a similar thing can happen. I've updated the PR to simply reallocate the shared memory area only if the size has changed. This change introduces some refactoring along the lines of #200. It doesn't however implement the additional logging of that PR. |
Contributor
|
@matt335672 This seems like a win to me, but let's make sure we put neutrinolabs/xrdp#1928 to bed before we merge this. |
Refactor -- Update name and reduce nesting.
Switch name.
metalefty
added a commit
that referenced
this pull request
Sep 7, 2022
[v0.9] Ignore screen size changes which don't change anything (#203)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As a result of investigating neutrinolabs/xrdp#1928, it was discovered that that when
mstsc.exehas a single monitor session full-screen, and this session is moved to another monitor and made full-screen,mstsc.exesends two seemingly identical DISPLAYCONTROL_MONITOR_LAYOUT_PDUs for the new screen size in quick succession.This behaviour was observed on
mstsc.exeversion 10.0.19041.1266. This doesn't seem to be necessary, but we need to deal with it.[MS-RDPEDISP] has nothing to say on this, except that the server should react to valid messages.
In practice, what happens is that the two messages cause xorgxrdp to reallocate the shared memory area twice. Between the first allocation and the second allocation, an update is send to xup with the id of the first shared memory area. If xup does not process this mesage until after the second area has been allocated, xup fails to connect to the shared memory area, and xrdp exits.
See also neutrinolabs/xrdp#2065 which attempts to improve xrdp logging in this area.
This PR deals with the above situation by only reallocating the shared memory area if it differs in size from the previous area. In use cases like that mentioned in neutrinolabs/xrdp#1928, where a single-monitor session is moved from one monitor to a monitor of the same resolution, this reallocation will be unnecessary anyway.
@Nexarian - I'd very much appreciate your comments on this, particularly as to whether this is the right way to deal with this. I'll be happy to answer any questions you might have.