Hi Idan, There are many things which deserve to be improved regarding the reconnection, but I don't think merging "connect" and "reconnect" functions is one of them. However, you made me look at the code and realize that the recent addition of autoreconnection indeed created functions similar in functionality that would deserve to be merged.
The reason why I think we need an explicit reconnect function is because we really need to ensure we are resetting the state of all variables, data structures, etc, properly before connecting again. For redirection, rdp_client_redirect takes care of doing that before calling rdp_client_connect. In other words, there's no duplication of code for the connection part, both use the same, but the "re" function does all the cleanup and resetting first. Now what I noticed is that the autoreconnection functionality uses freerdp_reconnect which uses in return rdp_client_reconnect. This function shares a lot of the context resetting that rdp_client_redirect has to do, but there's a bunch of stuff which is missing there. It explains issue #1818: https://github.com/FreeRDP/FreeRDP/issues/1818 I'll see if I can work on merging those two to really have one function handle all the common context resetting today. On Thu, May 1, 2014 at 2:33 AM, Idan Freiberg <spe...@gmail.com> wrote: > Hi Marc, > > Thanks for the update. > > I dont like that design when we use rdp_reconnect as a seperate function, > for second connection. > > What do you think about merging it to the rdp_connect function so well not > have to maintain them both? > > Maybe we can do full cleanup and jump again to connect. > On May 1, 2014 1:00 AM, "Marc-André Moreau" <marcandre.mor...@gmail.com> > wrote: > >> Hi Idan, >> >> I just found another problem, which I fixed here: >> >> https://github.com/awakecoding/FreeRDP/commit/5b0822a43769c7e00fdbdbd0e3a2af83a8e85dfe >> >> On reconnection, we weren't getting rid of the initial >> settings->LoadBalanceInfo, which caused it to be reused even if the >> redirected packet did not contain it. I modified the code to free it and >> set it to null so it doesn't get reused when redirected without the >> LoadBalanceInfo. I'm now getting further, but the test environment I have >> access to is behind a gateway, and I'm thinking there's another issue >> involving the gateway. >> >> There's a chance you get better results after this fix in your scenario >> which does not involve the gateway. >> >> To summarize, here is how this particular test environment is setup: >> >> There is a connection broker behind a gateway. You connect first to that >> broker with an initial LoadBalanceInfo string. The broker sends a >> redirection packet, which is IP-based, not cookie-based. The client then >> reconnects through the gateway but targeting that new host which is behind >> the gateway. The problem I previously had was that the original >> LoadBalanceInfo string was reused for that redirection session when it >> shouldn't have been, causing it to hang. Now it seems to correctly do the >> X224 negotiation, but it hangs, and I'm wondering if this isn't a problem >> with the gateway code and reconnection. If I manually start a new >> connection from scratch with the redirected information is connects >> properly though, which is why I'm thinking there might be a problem in the >> reconnection code for reinitializing some other variables. >> >> >> >> On Wed, Apr 30, 2014 at 12:53 PM, Idan Freiberg <spe...@gmail.com> wrote: >> >>> Hi, >>> >>> Afak, you shouldn't connect to the connection broker directly. >>> The process is : >>> 1. Connect to a RDSH server which is a farm member. >>> 2. That server should query the broker whether to connect you to this >>> server or another. >>> 3. If the broker says that you may connect to another RDSH, then the >>> first server is sending back an Server Redirection packet to mstsc client. >>> 4. If redirected, New rdp connection is set. >>> On Apr 30, 2014 5:46 PM, "Marc-André Moreau" <marcandre.mor...@gmail.com> >>> wrote: >>> >>>> Hi Alexis, >>>> >>>> From my understanding, your deployment does not use a TS Gateway, which >>>> should make debugging easier for us. Knowing this, here's what you can >>>> do: >>>> >>>> 1) Get mstsc to connect through the broker correctly >>>> 2) Capture traffic with wireshark while mstsc connects >>>> >>>> There should be two TCP connections done by mstsc. The first connection >>>> is >>>> done with the broker, and the broker sends a redirection packet to the >>>> client. The client then disconnects and reconnects based on that >>>> redirection packet. Information related to connection brokering is >>>> present >>>> in the very first packet sent over TCP in an RDP connection, the X224 >>>> Connection Request. In wireshark, look for the beginning of of those tcp >>>> connections, and copy those X224 connection requests. I'd like to >>>> inspect >>>> them, as the specs are very vague as to what should happen in practice >>>> for >>>> brokering. >>>> >>>> In addition to this, if you can have FreeRDP build with >>>> WITH_DEBUG_REDIR, >>>> I'd have a sample redirection packet to look at. From there we can try >>>> to >>>> figure out what FreeRDP is doing differently from mstsc. >>>> >>>> >>>> On Wed, Apr 30, 2014 at 9:17 AM, Marc-André Moreau < >>>> marcandre.mor...@gmail.com> wrote: >>>> >>>> > Hi Alexis, >>>> > >>>> > You can build with WITH_DEBUG_REDIR, and see what redirection packet >>>> you >>>> > are getting. You are getting the redirection packet, but reconnection >>>> to >>>> > the redirected machine fails. In the case of the person I was >>>> assisting the >>>> > other day, I had issues connecting with mstsc, so it was hard to >>>> figure out >>>> > what was truly wrong. Also, I assume this should be cookie-based >>>> > redirection, not ip-based redirection, correct? The main difference >>>> is that >>>> > with cookie based, the client reconnect to the broker a second time >>>> but >>>> > with a cookie that was provided in the redirection packet. In >>>> ip-based, the >>>> > client reconnects directly to the address provided in the redirection >>>> > packet. >>>> > >>>> > >>>> > On Wed, Apr 30, 2014 at 3:03 AM, Alexis Moinet < >>>> alexis.moi...@umons.ac.be>wrote: >>>> > >>>> >> Hello >>>> >> >>>> >> On 29/04/14 17:02, Marc-André Moreau wrote: >>>> >> > I've been investigating this all of yesterday with someone on IRC. >>>> I >>>> >> fixed >>>> >> > the double free issue on my branch. >>>> >> > >>>> >> >>>> https://github.com/awakecoding/FreeRDP/commit/c2a59c23a7b8eb262209bdadb66fe1e429ec943e >>>> >> >>>> >> Indeed, I can confirm no double free anymore, thanks! >>>> >> >>>> >> > This problem would only occur in the case of a failed redirection. >>>> >> >>>> >> $ ./xfreerdp -u username -d domain -v rdsserver >>>> >> connected to rdsserver:3389 >>>> >> Password: >>>> >> freerdp_set_last_error 0x2000C >>>> >> Error: protocol security negotiation or connection failure >>>> >> recv: Bad file descriptor >>>> >> >>>> >> what kind of debug info would be useful to figure this out ? or at >>>> least >>>> >> to know whether it's a freerdp bug or a server-side problem >>>> >> >>>> >> so far as I know, microsoft client is working fine, but I cannot >>>> test it >>>> >> right now. >>>> >> >>>> >> thanks, >>>> >> >>>> >> Alexis >>>> >> >>>> > >>>> > >>>> >>>> ------------------------------------------------------------------------------ >>>> "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE >>>> Instantly run your Selenium tests across 300+ browser/OS combos. Get >>>> unparalleled scalability from the best Selenium testing platform >>>> available. >>>> Simple to use. Nothing to install. Get started now for free." >>>> http://p.sf.net/sfu/SauceLabs >>>> _______________________________________________ >>>> Freerdp-devel mailing list >>>> Freerdp-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/freerdp-devel >>>> >>> >> ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available. Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ Freerdp-devel mailing list Freerdp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freerdp-devel
