Skip to content

guac fix#2903

Open
doomedraven wants to merge 7 commits intomasterfrom
guac_fix
Open

guac fix#2903
doomedraven wants to merge 7 commits intomasterfrom
guac_fix

Conversation

@doomedraven
Copy link
Collaborator

✦ I have resolved the reported issues in the Guacamole web interface integration.

   1. Fixed `guac-web.service` startup error: Updated web/web/asgi.py to explicitly register the http handler in ProtocolTypeRouter, complying with Channels 4.x requirements.
   2. Fixed "slow and unresponsive" session: In web/guac/consumers.py, wrapped blocking GuacamoleClient methods (handshake and send) with sync_to_async. This prevents the synchronous calls from blocking the ASGI
      event loop, which was causing the lag and crashes.
   3. Prevented infinite loops: Updated the open method in web/guac/consumers.py to correctly handle GuacamoleClient disconnection (EOF) by breaking the receive loop and closing the WebSocket, preventing high CPU
      usage on disconnect.

  The files web/web/asgi.py and web/guac/consumers.py have been updated and syntax checked.

```
✦ I have resolved the reported issues in the Guacamole web interface integration.

   1. Fixed `guac-web.service` startup error: Updated web/web/asgi.py to explicitly register the http handler in ProtocolTypeRouter, complying with Channels 4.x requirements.
   2. Fixed "slow and unresponsive" session: In web/guac/consumers.py, wrapped blocking GuacamoleClient methods (handshake and send) with sync_to_async. This prevents the synchronous calls from blocking the ASGI
      event loop, which was causing the lag and crashes.
   3. Prevented infinite loops: Updated the open method in web/guac/consumers.py to correctly handle GuacamoleClient disconnection (EOF) by breaking the receive loop and closing the WebSocket, preventing high CPU
      usage on disconnect.

  The files web/web/asgi.py and web/guac/consumers.py have been updated and syntax checked.

```
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @doomedraven, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the stability and performance of the Guacamole web interface integration. It resolves critical issues related to service startup, session responsiveness, and proper disconnection handling, ensuring a more robust and efficient user experience.

Highlights

  • Guacamole Service Startup Fix: Resolved an issue where the guac-web.service failed to start by explicitly registering the HTTP handler in the ProtocolTypeRouter within web/web/asgi.py, aligning with Channels 4.x requirements.
  • Improved Session Responsiveness: Addressed 'slow and unresponsive' Guacamole sessions by wrapping blocking GuacamoleClient methods (handshake and send) with sync_to_async in web/guac/consumers.py. This prevents synchronous calls from blocking the ASGI event loop, enhancing performance and stability.
  • Prevented Infinite Loops on Disconnect: Modified the open method in web/guac/consumers.py to correctly handle GuacamoleClient disconnections (EOF). The receive loop now breaks and the WebSocket closes upon disconnection, preventing high CPU usage and infinite loops.
Changelog
  • web/guac/consumers.py
    • Wrapped self.client.handshake with sync_to_async to prevent blocking the ASGI event loop during connection.
    • Wrapped self.client.send with sync_to_async to prevent blocking the ASGI event loop during data transmission.
    • Implemented a try...finally block in the open method to ensure self.close() is called.
    • Added logic to break the receive loop in the open method when GuacamoleClient.receive() returns no content, indicating a disconnection, to prevent infinite loops.
  • web/web/asgi.py
    • Added "http": django_asgi_app to the ProtocolTypeRouter to explicitly register the HTTP handler, resolving a service startup error related to Channels 4.x.
Activity
  • The pull request was created by doomedraven to address reported issues in the Guacamole web interface integration.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request provides crucial fixes for the Guacamole web interface integration. The changes correctly address startup errors, session performance issues, and potential infinite loops by adopting modern asynchronous practices with Django Channels and adding robust error handling. The use of sync_to_async for blocking calls and proper handling of connection lifecycle events are well-implemented. I've suggested a minor improvement to enhance logging within the message loop for better observability, but overall, the changes are excellent and significantly improve the stability and performance of the feature.

doomedraven and others added 3 commits February 6, 2026 08:11
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@doomedraven
Copy link
Collaborator Author

@ClaudioWayne can you test it now? you need to install dependencies and restart web server

@doomedraven
Copy link
Collaborator Author

@ClaudioWayne some extra interesting findings, but, you need to verify the guacamole version on your server, that is important, it might be obsolete

=====

Here is a summary of the investigation and fixes for the Guacamole connection failure after upgrading to Django Channels 4. You can post this message to your issue tracker, team chat, or documentation.


Summary: Fix for Guacamole "Error filling instruction buffer" (Channels 4 Migration)

The Issue
After upgrading channels from 3.x to 4.x, users experience a "Guacamole connection failure: Error filling instruction buffer."

Root Cause
This error occurs because the Guacamole client (JavaScript) expects a binary WebSocket stream but receives malformed data (often an HTML 500/403 error page or a raw HTTP response). This is caused by fundamental architectural changes in Channels 4:

  1. Daphne Decoupling: Channels 4 no longer automatically provides the ASGI server. Without explicit setup, the app runs as standard WSGI, failing the WebSocket upgrade.
  2. Stricter Handshakes: Channels 4 requires explicit definition of WebSocket subprotocols.
  3. Async Changes: Deprecated asyncio patterns in the consumer caused runtime crashes.

The Solution
We have applied the following fixes to migrate the codebase to Channels 4 standards:

1. Dependency & Settings (settings.py)

  • Action: Explicitly added 'daphne' to INSTALLED_APPS at the top of the list.
  • Reason: Ensures the runserver command loads the ASGI interface capable of handling WebSockets.

2. ASGI Entry Point (asgi.py)

  • Action: Refactored to use django.core.asgi.get_asgi_application() instead of the removed AsgiHandler.
  • Action: Fixed import ordering to ensure sys.path is set before importing local modules.
  • Action: Wrapped routing in AllowedHostsOriginValidator.
  • Reason: Prevents 403 Forbidden errors (which confuse the Guacamole client) and ensures the app loads correctly.

3. Consumer Logic (consumers.py)

  • Action: Added subprotocols = ["guacamole"] to the GuacamoleWebSocketConsumer class.
  • Action: Replaced loop.create_task with asyncio.create_task (Python 3.10+ compliance).
  • Action: Added try/except blocks around the connect() logic.
  • Reason: Explicitly negotiating the "guacamole" subprotocol is now mandatory. The error handling ensures that if a connection fails, the socket closes cleanly rather than leaking HTML error pages to the binary client.

Status
These changes fully align the application with the Channels 4 architecture and restore Guacamole connectivity.

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

Successfully merging this pull request may close these issues.

1 participant