Can one of the admins verify this patch?

I am checked transport_layer_bio_ctrl.
What I understand is that BIO_wait_read should be used here as a better abstraction.(wait for stuff other than the socket).
if oter than the socket, it may not event. call layer wait is better.
I don't understand what you mean by abstract. please Please give an example.

@KangLin Consider the direct connection (no gateway):

• BIO_wait_read will eventually call freerdp_tcp_layer_wait and block there in poll. no way to wait for abortEvent et al.
• freerdp_tcp_layer_get_event will wait for freerdp_tcp_connect_layer to set the WSAEvent (which wraps native sockets) and can be used to wait for multiple handles

so, this sounds like you need your own implementation of that layer based on QTcpSocket?
you can wrap that the same way as current native socket implementation, just set your own transport_default_connect_layer?

@KangLin Consider the direct connection (no gateway):

* `BIO_wait_read` will eventually call `freerdp_tcp_layer_wait` and block there in `poll`. no way to wait for abortEvent et al.

yes. it isn't wait abortEVENT in here.
so to use BIO_wait_read is better

* `freerdp_tcp_layer_get_event` will wait for `freerdp_tcp_connect_layer` to set the `WSAEvent` (which wraps native sockets) and can be used to wait for multiple handles

It's used for the main event loop, but it's not suitable for use here.

so, this sounds like you need your own implementation of that layer based on QTcpSocket? you can wrap that the same way as current native socket implementation, just set your own transport_default_connect_layer?

Yes. see:

• https://github.com/KangLin/RabbitRemoteControl/blob/develop/Plugins/FreeRDP/Client/ConnectLayerQTcpSocket.cpp
• https://github.com/KangLin/RabbitRemoteControl/blob/develop/Plugins/FreeRDP/Client/ConnectLayer.cpp

TLS should not block here alone. 0 should be returned directly and then from the main event loop into state machine.

@KangLin do I see this correct that you try to open a socket and then pass that to FreeRDP?

that is already available with default implementation, see freerdp_tcp_default_connect
[edit] documentation is not good for this, so here is an example: /v:|:1234 will directly use the socket 1234

TLS should not block here alone. 0 should be returned directly and then from the main event loop into state machine.

well, it will block if your connection has high latency for example

@KangLin Consider the direct connection (no gateway):

* `BIO_wait_read` will eventually call `freerdp_tcp_layer_wait` and block there in `poll`. no way to wait for abortEvent et al.

yes. it isn't wait abortEVENT in here. so to use BIO_wait_read is better

but you can combine the HANDLE with that to check for changes.

* `freerdp_tcp_layer_get_event` will wait for `freerdp_tcp_connect_layer` to set the `WSAEvent` (which wraps native sockets) and can be used to wait for multiple handles

It's used for the main event loop, but it's not suitable for use here.

there is no main event loop during connect, just the blocking call.
we're reworking stuff to make it less blocking (so you can actually abort properly during connect without a timeout)

@KangLin do I see this correct that you try to open a socket and then pass that to FreeRDP?

that is already available with default implementation, see freerdp_tcp_default_connect [edit] documentation is not good for this, so here is an example: /v:|:1234 will directly use the socket 1234

@akallabeth

• There is no problem using pipes and unix sockets. CChannelSSHTunnelForward
• I implemented a connection layer based on libssh: ConnectLayerSSHTunnel. It has a nation socket. so using event and wait read is the same.
• I implemented a connection layer based on QTcpSocket: CConnectLayerQTcpSocket. However, QTcpSocket may not be converted to event, so in this case you can only use wait read.

@akallabeth

typedef struct
{
		ALIGN64 void* userContext;
		ALIGN64 pTransportLayerRead Read;
		ALIGN64 pTransportLayerWrite Write;
		ALIGN64 pTransportLayerFkt Close;
		ALIGN64 pTransportLayerWait Wait;
		ALIGN64 pTransportLayerGetEvent GetEvent;
		UINT64 reserved[64 - 6]; /* Reserve some space for ABI compatibility */
} rdpTransportLayer;

rdpTransportLayer

The rdpTransportLayer.GetEvent is used for noblocking.
The rdpTransportLayer.Wait is used for blocking.

So rdpTransportLayer.Wait is better than rdpTransportLayer.GetEvent here.

@KangLin the opposite is the case.

We're trying to remove all locations where you need a blocking (unconditional) wait that can not be broken by freerdp_abort_connect_context.

Here we have a blocking loop with a timeout.
the timeout should be eliminated and a infinite wait used instead (with the aforementioned abort condition)
Only if that event is triggered the read should be done, not before.

(yes, it is currently not 100% consistent, but the above is the idea that the implementation should conform to)

in any case, if you have a problem in your implementation, then I'd check that first, as mentioned before, the current implementation can already reuse an already opened socket without any modifications.

@akallabeth

in any case, if you have a problem in your implementation, then I'd check that first, as mentioned before, the current implementation can already reuse an already opened socket without any modifications.

There is nothing problem with my current implementation.

We're trying to remove all locations where you need a blocking (unconditional) wait that can not be broken by freerdp_abort_connect_context.

Here we have a blocking loop with a timeout.
the timeout should be eliminated and a infinite wait used instead (with the aforementioned abort condition)
Only if that event is triggered the read should be done, not before.

#11651 I don't think this solves the problem.

I think this is a program framework design problem.
I think pollAndHandshake() and similar blocking functions should not be provided.

I think the final approach is the design should be based on event-driven. No need to block internally.

When data cannot be read, it returns directly to the main event loop and waits for the next time data can be read, and then executes according to the current status again.

The current BIO design has two layers, the upper layer is the memory cache and the lower layer is the connection layer.
When the data to be read is larger than the data in the cache, you can directly return(don't blocking) and enter the main event loop.