xpra icon
Bug tracker and wiki

Opened 2 weeks ago

Last modified 35 hours ago

#1838 assigned enhancement

completely skip base classes

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

Description (last modified by Antoine Martin)

Similar to #1836 but for mixins (#1778 / #1761), we should be able to completely skip some of the server base classes.
ie:

  • if remote-logging is disabled, don't inherit from LoggingServer
  • if mmap is disabled, don't inherit from MMAP_Server

etc

This would reduce the memory footprint, and increase the security (decreasing the attack surface): it is impossible to attack code which isn't there.

Could be done for the server by using a dynamic type for "server base" (see example patch).
The client is not modular enough to support this sort of refactoring. (see #1796 for authentication handlers)

Attachments (1)

bases.patch (4.1 KB) - added by Antoine Martin 2 weeks ago.
dynamic list of base classes

Download all attachments as: .zip

Change History (6)

Changed 2 weeks ago by Antoine Martin

Attachment: bases.patch added

dynamic list of base classes

comment:1 Changed 2 weeks ago by Antoine Martin

Description: modified (diff)
Status: newassigned

Huge commits making things modular in r19283 + r19282.

This may help with #1123, we just need a new "upgrade" argument for the cleanup method.

comment:2 Changed 2 weeks ago by Antoine Martin

As of r19284, the server can completely skip a number of modules:

xpra start --notifications=no --webcam=no --speaker=no --microphone=no \
    --file-transfer=no --printing=no --dbus-proxy=no \
    --remote-logging=no --mmap=no --clipboard=no --av-sync=no

(this will disable most of the optional mixins, both in the server base class and in the "client connection" instance)

Still TODO:

  • add test script running every combination possible (there are 2**10=1024!) to check for invalid attribute dependencies
  • ideally, deal with the "mmap_size" check more cleanly - meh
  • add switch for "idle", "client info" mixins
  • make control commands mixin optional (some subclasses assume this is always defined)
  • make it possible to disable the "commands", "encoding" and "network state" switches? (hard, especially for "encoding")
  • this probably caused #1841
Last edited 11 days ago by Antoine Martin (previous) (diff)

comment:3 Changed 12 days ago by Antoine Martin

Regression: #1841, fixup: r19363

Last edited 4 days ago by Antoine Martin (previous) (diff)

comment:4 Changed 3 days ago by Antoine Martin

Automated test script added in r19371, with some bug fixes included.

This script exposes lots of ugly dependency issues. (in particular with read-only mode, keyboard, etc)
So many more bugs are left to fix..

comment:5 Changed 3 days ago by Antoine Martin

Updates:

  • r19373 + r19381 + r19388 + r19389 fix readonly mode
  • r19400 skip shadow pointer polling when input devices are disabled
  • r19374 client bug when the desktop-size is not provided
  • r19375 fix wrong location for packet-queue
  • r19401 window model management refactoring
  • r19376 improve client warning message when the server does not forward any windows (--windows=no)
  • r19379 + r19395 + r19406 major unit tests improvements
  • fixes: r19380

Still TODO:

  • send_cursor might belong to the display rather than windows mixin?
  • the "desktop_size_unscaled" / "desktop_size" code in x11 server base is a bit ugly
  • same for "suspended" attribute used in source mixins: r19409
  • we should also test shadow servers, desktop servers, and vnc clients
  • maybe verify that the window we forward is on the client's vfb screen (checking pixel colour at window location)
  • use super() more
  • batch delay and other data is missing from session info
Last edited 35 hours ago by Antoine Martin (previous) (diff)
Note: See TracTickets for help on using tickets.