Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

completely skip server base classes #1838

Closed
totaam opened this issue May 11, 2018 · 13 comments
Closed

completely skip server base classes #1838

totaam opened this issue May 11, 2018 · 13 comments

Comments

@totaam
Copy link
Collaborator

totaam commented May 11, 2018

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)

@totaam
Copy link
Collaborator Author

totaam commented May 11, 2018

Huge commits making things modular in r19283 + r19282.

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

@totaam
Copy link
Collaborator Author

totaam commented May 11, 2018

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 server display size not adjusted #1841

@totaam
Copy link
Collaborator Author

totaam commented May 13, 2018

Regression: #1841, fixup: r19363

@totaam
Copy link
Collaborator Author

totaam commented May 22, 2018

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..

@totaam
Copy link
Collaborator Author

totaam commented May 23, 2018

Updates:

Still TODO:

  • send_cursor might belong in 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
  • keys_pressed should be moved to the keyboard config: r19466
  • maybe verify that the window we forward is on the client's vfb screen (checking pixel colour at window location)
  • use super() more
  • add new build options to exclude some subsystems completely and avoid failures when they cannot be imported
  • batch delay and other data is missing from session info

@totaam
Copy link
Collaborator Author

totaam commented May 29, 2018

For the client, see #1861

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2018

Ugly dependency utility cursor functions added: #1658#comment:8.

@totaam
Copy link
Collaborator Author

totaam commented Jun 26, 2018

  • as of r19723 + r19724, we can run the server with some modules completely missing. Either not installing them (ie: build with --without-dbus - new switches added in r19725) or by nuking them afterwards (ie: rm -fr /usr/lib64/python2.7/site-packages/xpra/keyboard/).

This is true for the following xpra submodules: notifications, keyboard, clipboard, sound and dbus.
The server runs but prints a warning if the settings or command line try to enable functionality that is not available, ie:

Warning: missing notifications module
  • r19726: the server can run without any codecs at all, this also means without any webcam or window forwarding
  • r19727: the client avoids sending keyboard and pointer events if the server doesn't know how to handle them
  • r19728: the client still runs without any of the optional modules (but those are still loaded - not the same as skip client base classes #1861)

@totaam
Copy link
Collaborator Author

totaam commented Jul 9, 2018

@maxmylyn: FYI, feel free to close. (see comment:8)

Will follow up in #1913 (high memory usage).

@totaam totaam closed this as completed Jul 9, 2018
@totaam
Copy link
Collaborator Author

totaam commented Sep 3, 2018

r20282 does the same for RFB in shadow and desktop servers.

@totaam
Copy link
Collaborator Author

totaam commented Sep 3, 2018

norman commented:

Would you backport a fix to the v2.3.x branch (and maybe others?) to include calls to IdleMixin and ClipboardConnection in ClientConnection.get_info? I noticed that the data is missing :-(

@totaam
Copy link
Collaborator Author

totaam commented Sep 3, 2018

Would you backport a fix to the v2.3.x branch (and maybe others?) to include calls to IdleMixin and ClipboardConnection in ClientConnection.get_info? I noticed that the data is missing :-(

Done in 20284.
@norman: does that work for you?

@totaam
Copy link
Collaborator Author

totaam commented Jun 29, 2019

See also #2344 and #2351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant