xpra icon
Bug tracker and wiki

This bug tracker and wiki are being discontinued
please use https://github.com/Xpra-org/xpra instead.

Opened 3 years ago

Closed 2 years ago

Last modified 17 months ago

#2351 closed enhancement (fixed)

dynamic client connection class

Reported by: Antoine Martin Owned by: Antoine Martin
Priority: major Milestone: 4.0
Component: server Version: 2.5.x
Keywords: Cc:


Taking #1838 one step further: when the client disables a feature completely (ie: clipboard) or when the client is not a UI client (ie: #2348) then we can generate a custom client class without those base classes.

get_client_connection_class needs to be made dynamic.
We'll need a map from flags to mixins, ie:

 "clipboard" : ClipboardConnection,
 "webcam" : WebcamMixin,

Attachments (1)

skip-clientconnection-mixins.patch (15.7 KB) - added by Antoine Martin 2 years ago.
implementation - causes server hangs on control-c

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by Antoine Martin

Status: newassigned

Preparatory work in r23081.

comment:2 Changed 3 years ago by Antoine Martin

Milestone: 3.04.0

This will be much easier to implement after dropping python2 support. (v4)

Changed 2 years ago by Antoine Martin

implementation - causes server hangs on control-c

comment:3 Changed 2 years ago by Antoine Martin

No idea why, but re-doing the change step by step no longer crashes and so the code has been merged in r24907.
The only mixin we currently skip is the mmap mixin.

Still TODO:

  • add the is_needed check to more mixins
  • try to minimize module level imports so that the is_needed check does not cost too much
  • in some cases, add a new flag so we can distinguish between something that is available but disabled from something that is not available (ie: av-sync could start turned off and be enabled via a control command, clipboard)

comment:4 Changed 2 years ago by Antoine Martin

Much improved in r24910, see commit message for details. With minor updates in r24911 + r24912 + r24913 + r24914.
Minimal backport to help the server figure out what the client is capable of: r24915.
With a regular client and xpra top connected:

$ xpra info | grep ".modules"
client.1.modules=('Client', 'ClientInfo', 'Clipboard', 'Audio', 'Webcam', 'FilePrint', 'MMAP', 'Input', 'DBUS', 'NetworkState', 'ClientDisplay', 'Windows', 'Encodings', 'AVSync', 'Idle')
client.modules=('Client', 'ClientInfo', 'Webcam', 'Input', 'DBUS', 'NetworkState', 'ClientDisplay', 'Idle')

Apart from the stated goals of not loading code we don't need (reducing attack surface, reducing memory usage, etc), there are additional benefits: we are forced to remove cross-mixin dependencies and this reduces contention on shared resources. ie: a client which has disabled the clipboard will make it easier for other users.

Some caveats.

  • av-sync: older clients will send "av-sync" properties even when disabled... so with those we enable the mixin even when not needed - could backport this trivial fix
  • display mixin doesn't have any clear on-off properties
  • input mixin needs disentangling
  • webcam lacked a client capability attribute... so we should add a separate client flag to workaround that (ie: "webcam.mixin.caps")

Still TODO: test all possible combinations (one of the unit tests can do that - just disabled by default because it takes so long)

Last edited 2 years ago by Antoine Martin (previous) (diff)

comment:5 Changed 2 years ago by Antoine Martin

Resolution: fixed
Status: assignedclosed

Unless needed skip those mixins:

This allows the "top" client to load only the bare minimum:

client.modules=('Client', 'ClientInfo', 'ClientDisplay')

This will do for v4.

comment:6 Changed 17 months ago by migration script

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/2351

Note: See TracTickets for help on using tickets.