Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    metalefty
    @metalefty
    vsock support was added by MS guys for Hyper-V quick create. If they abandon vsock, we may be remove it in future.
    in short term, i don’t have any plan to remove vsock but if it become unmaintainable it might be removed.
    Nexarian
    @Nexarian
    I wish the XRDP community had a contact/mole/resource within Microsoft. Coldly reimplementing the spec is annoying.
    arops2020
    @ar2020-devops
    Hello Team
    Need some help on some information

    I am Looking for setting up a message in Banner for Linux server while loggin usinf xrdp. I tried to add my banner msg in this file /etc/xrdp/xrdp.ini :

    ; Login Screen Window Title
    ls_title=

    But its no success. I have restarted the xrdp service as well. Any help/Suggestion. Thanks and Advance.

    jsomiak1
    @jsomiak1
    hi, i need lil information, i need to know its anyway for see my incoming connections to my rdp, where the connection from?
    Nexarian
    @Nexarian
    Made more progress on dynamic monitor last night. I suspect the crashes I'm seeing are because xrdp and xorgxrdp gets out of sync
    So I need to either re-use an existing way to keep them in sync, or add one.
    Nexarian
    @Nexarian
    I think this is what I need to use: neutrinolabs/xrdp@21f90e3
    Nexarian
    @Nexarian

    Ugh, trying it does indeed seem to help, but it breaks the Microsoft Remote Desktop client, so must keep digging.

    Does anyone have tips on debugging why something might work with FreeRDP but not with the Microsoft versions?

    matt335672
    @matt335672
    No particular tips on debugging that I'm afraid.
    A lot of your crashes seem to be related to the timer callback (IIRC). Is it possible that some of those callbacks are working with stale info, and that's causing your crashes? I'm away from a development environment at the moment I'm afraid, so working from memory.
    Nexarian
    @Nexarian

    I'm trying to think how I might force the callbacks to work with up-to-date memory. Essentially this is a pretty fundamental shift for the driver in this regard. The size of video memory could shift at any time, whereas before a baked-in assumption was that it would only change during connection time.

    So -- How to fix that? A couple ideas off the top of my head:

    1. Protect the relevant memory with a mutex and an interrupt signal. Fire the interrupt on any changes which will prompt the capture callback to release the mutex, and no other callbacks will be allowed to access the memory due to the resizing algorithm consuming the mutex.
    2. Add checks within the capture callback to make sure that it's always using up-to-date memory.
    3. My attempt at using suppress sorta kinda helped in some circumstances, but because this is a timer callback it's operating on a different thread, which means I can't force the data to always be consistent.
    4. Protect every interaction with the memory buffer with a size check instead of a mutex -- This seems more extreme and probably would impact performance.
    Nexarian
    @Nexarian

    Then again, the entire reason the suppress output was created was to prevent the driver from writing to memory when the window was minimized -- Because that narrow use-case is in effect a resolution change, but under a very specific circumstance.

    Fun fact: In Windows 95, it never actually minimized the windows. Instead, it just moved them off to (MAX_INT, MAX_INT) in virtual video memory. If you ever had a display that could show that much resolution, you would never see anything minimize, so there were some people who came up with hacks, like scaling the desktop in the extreme, to be able to see it :)

    aquesnel
    @aquesnel
    I don't know the driver code at all, so this is an uninformed suggestion.
    Does it make sense to use like a double buffering strategy? The driver only write to one buffer and then send a message to switch buffers. You could only send the message to switch buffers when there is a resolution change.
    Nexarian
    @Nexarian

    I also don't know the driver code at all, I'm largely just railing against this out of pure passion/tenacity. I think that the lack of overall knowledge as to how xorgxrdp works something the XRDP community could use an improvement on (and I guess I'm trying to improve it by helping out myself)

    But I do like that strategy! I suspect it might be a rather large undertaking as the buffer in question may in fact be aliased to something internally represented in the X Window system, but X itself may also support that. But yeah, all of this is speculation at this point.

    At present I'm having a spot of trouble reproducing the issue consistently. It seems to reproduce most clearly over WAN connections with some latency (which makes sense since this is invoked by timer). On my local desktop, getting it to crash is really hard because I have two VirtualBox Ubuntu VMs connecting to each other, and the latency on that is nearly 0, and the resizing is rock solid. I can't get it to crash in spite of doing lots of crazy resizing.

    My next step is to test out with WAN connections the "suppress" flag and the "resize" flag that Jay added last year and see if either of them help the issue.

    I actually suspect the resize flag that was added last year: neutrinolabs/xorgxrdp@228e909 may have been for a very similar issue. The problem was that multiple callbacks for resize were being invoked at inopportune times, and this flag is essentially a global notification of "hold up, resizing, please wait"
    Nexarian
    @Nexarian
    But the good news is we may already have a video memory copy routine that could work here: https://github.com/neutrinolabs/xorgxrdp/blob/devel/module/rdpCapture.c#L1080
    Nexarian
    @Nexarian

    I think I've got it! The fix was indeed to move suppress around

    https://github.com/Nexarian/xrdp/tree/dynamic_monitor_xrdp_master

    Give this a whirl with FreeRDP /usr/local/bin/xfreerdp /bpp:32 /v:c[hostname]:3389 /network:lan /sound /u:[user] /p:[password] /rfx /dynamic-resolution /w:2560 /h:1396

    Note that the height is calibrated for Mac OS X Catalina on a 1440p display, it takes into account the height of the title and menu bar (assuming you have the dock minimized)

    1 reply
    Nexarian
    @Nexarian
    If I'm right, the fix was stupidly simple: Nexarian/xrdp@a74a0c5
    matt335672
    @matt335672

    Seems like a reasonable approach, but it still leavers us with a fragile xorgxrdp driver.

    A couple of other possibilities:-

    1. Have a global counter in xorgxrdp which is incremented on a resize. The callback can then check that it's working with the same video memory layout, and simply finish if it isn't.
    2. If a resize is detected in the video driver when output isn't suppressed, log a significant warning so we can track back and fix it later on.

    Hope that's useful.

    8 replies
    aquesnel
    @aquesnel
    @metalefty can you please take another look at #1767 since I have updated the pull request since your last comment. Thanks.
    metalefty
    @metalefty
    @aquesnel approved.
    Nexarian
    @Nexarian
    @matt335672 So I got a crash even with my fancy suppress output fix. But after a bit more intense debugging I think I figured out WHY it's crashing. I've posted details in the issue: neutrinolabs/xrdp#448
    Nexarian
    @Nexarian

    @metalefty @matt335672 Here's my first PR! I figured out the issue with xorgxrdp, and this should fix the issue and lock it up tight: neutrinolabs/xorgxrdp#183

    This change will help with overall driver stability regardless of the dynamic resizing, but it should be viewed as a pre-req to checking in the xrdp resizing feature.

    matt335672
    @matt335672
    Sounds very much like you're on the right track with this! I'll get round to a more significant review when I'm able.
    matt335672
    @matt335672

    General question - At the moment we've got a client_info structure in common/xrdp_client_info.h shared between xrdp and xorgxrdp. At the moment if we make an incompatible change to this structure in xrdp, users need to update xorgxrdp as well. If they don't, strange things happen with no real clues.

    Would it be a reasonable idea to add a guard value as the second longword here, so that we can pick up incompatible changes between the two and flag them in the driver? I was thinking of something like this:-

    struct xrdp_client_info
    {
      int size; /* bytes for this structure */
      int version; /* should be CLIENT_INFO_CURRENT_VERSION */
       . . .
    };
    . . .
    #define CLIENT_INFO_CURRENT_VERSION 20210225 /* yyyymmdd of last incompatible change */

    and in xrdp_rdp_create():-

        self->client_info.size = sizeof(self->client_info);
        self->client_info.version = CLIENT_INFO_CURRENT_VERSION;

    then in xorgxrdp:rdpClientConProcessMsgClientInfo()

    if (clientCon->client_info.version != CLIENT_INFO_CURRENT_VERSION)
    {
        LLOGLN(0, ("Incompatible client_info version detected (expected %d, got %d). Xorg is unlikely to work properly", CLIENT_INFO_CURRENT_VERSION, clientCon->client_info.version);
    }

    This is off the top of my head, BTW.

    If we make compatible changes to client_info, there's no need to bump the version number.

    Any thoughts?

    metalefty
    @metalefty
    I think that is a very good idea to detect xrdp/xorgxrdp version matches.
    xrdp.ini also has ini_version parameter to do a similar thing. actually, ini_version has never bumped yet.
    aquesnel
    @aquesnel
    I would encourage failing fast when a version mismatch is detected so that it is obvious that something is wrong.
    If there is a way for xrdp to get the version from xorgxrdp, that would allow sending a message to the user logging in and not just to the log file, which would make debugging version issues even easier.
    3 replies
    aquesnel
    @aquesnel
    As I've been re-doing the logging I've been fixing up the code style with astyle for all the files I've touched..
    I've also created a CI build step for checking the code formatting
    diff: https://github.com/neutrinolabs/xrdp/compare/devel...aquesnel:check_formatting
    example run: https://github.com/aquesnel/xrdp/runs/2000423635
    aquesnel
    @aquesnel
    Does anyone have feedback for eventually adding this code formatting check to the pull request CI builds once all the files have been updated to meet the code formatting style.
    Nexarian
    @Nexarian
    In my opinion, the formatting doesn't matter as long as we take steps to make it consistent.
    Because "good formatting" is arbitrary.
    Anyway, here's how to get dynamic resizing working on VNC! Nexarian/xrdp@f40451f
    matt335672
    @matt335672
    I've just raised neutrinolabs/xrdp#1816 as a placeholder for improving the Xorg backend startup. Comments welcome. Have I got the scope right?
    5 replies
    aquesnel
    @aquesnel
    #1816 looks good to me
    Nexarian
    @Nexarian

    I suspect based on the new PR I just submitted to XRDP regarding dynamic resize that something has broken with the cppcheck infrastructure: neutrinolabs/xrdp#1820

    Comparing the logs from an older commit to the one from this, it looks like something has changed with the libz3 installation.

    metalefty
    @metalefty
    Related to #1818, I wrote down my thougts on platform support policy. Just to be sure that we don't need to take special effort on m68k, s390 and sh processors even if #1818 is merged.
    I think Environment section in README is the only part we mention target platform so far. The purpose of the draft wiki page is make it clearer.
    https://github.com/neutrinolabs/xrdp/blob/f947843c2145b6770250f56155d34b2bcf44855d/README.md#environment
    matt335672
    @matt335672
    @metalefty - that looks like a reasonable statement to me.
    1 reply
    @Nexarian - I'll have a look at the cppcheck issue and figure out what's going on. Today's a little awkward but I'll see how far I can get.
    matt335672
    @matt335672

    cppcheck issue is (I think) caused by ubuntu-latest migrating to focal, but the cached cppcheck is still built with 18.04.

    See Z3Prover/z3#660

    Possible solutions:-

    • Get the OS version in the cache tag name. I haven't found a simple way to do this, but I've not been looking for long.
    • Move the cppcheck VM to a specific OS version rather than 'ubuntu-latest'

    Thoughts anyone?

    matt335672
    @matt335672
    Already raised by others, but no obvious solution yet.
    actions/cache#543
    matt335672
    @matt335672
    Fix for cppcheck - see neutrinolabs/xrdp#1821 for discussion
    1 reply