Ignore screen size changes which don't change anything #203
Conversation
This file contains 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
matt335672
force-pushed
the
resize_to_same_size
branch
from
eca1d70
to
6ad2724
Compare
|
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. |
|
@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
pushed a commit
to metalefty/xorgxrdp
that referenced
this pull request
Sep 6, 2022
) * Only reallocate shared memory if the size changes Co-authored-by: Nexarian <[email protected]>
metalefty
added a commit
that referenced
this pull request
Sep 7, 2022
[v0.9] Ignore screen size changes which don't change anything (#203)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.