new version full rebuild with claude opus 4.6 and own rdp daemon
This commit is contained in:
felixg
240
docker/patch-display-flush.py
Normal file
240
docker/patch-display-flush.py
Normal file
@@ -0,0 +1,240 @@
|
||||
#!/usr/bin/env python3
|
||||
"""
|
||||
Patch guacamole-server source files to fix bugs in git main that abort()
|
||||
or segfault the child process in debug builds (no -DNDEBUG).
|
||||
|
||||
--- Fix 1: src/libguac/display-flush.c ---
|
||||
|
||||
The frame-flush loop iterates over display layers to diff pending vs last frame.
|
||||
When a layer has pending_frame.buffer == NULL (e.g. cursor/aux layers that have
|
||||
never been drawn), the code does:
|
||||
|
||||
GUAC_ASSERT(current->pending_frame.buffer_is_external);
|
||||
continue; <- BUG: never advances `current`
|
||||
|
||||
Two problems:
|
||||
1. GUAC_ASSERT aborts the process if buffer_is_external is false, which happens
|
||||
for any fresh layer that has not yet received display data. In a debug build
|
||||
(no -DNDEBUG) this is a real assert -> abort() -> child process dies.
|
||||
2. If the assert passes, `continue` loops forever on the same layer.
|
||||
|
||||
Fix: replace the GUAC_ASSERT with a proper pointer advance before the continue.
|
||||
|
||||
--- Fix 2: src/libguac/display-plan.c ---
|
||||
|
||||
Identical bug: the same GUAC_ASSERT(current->pending_frame.buffer_is_external)
|
||||
pattern exists in display-plan.c (same layer-iteration loop, same missing pointer
|
||||
advance before continue). Applying the same fix.
|
||||
|
||||
--- Fix 3: src/libguac/user.c ---
|
||||
|
||||
guac_user_supports_webp() iterates user->info.image_mimetypes without checking
|
||||
whether the pointer itself is NULL. If the client did not send any image MIME
|
||||
types (or guac_copy_mimetypes() returned NULL), the very first dereference
|
||||
`*mimetype` causes a SIGSEGV in the display worker thread.
|
||||
|
||||
Fix: insert a NULL guard immediately after the pointer is loaded from the struct.
|
||||
|
||||
--- Fix 6: src/protocols/rdp/gdi.c (guac_rdp_gdi_end_paint) ---
|
||||
|
||||
With disable-gfx/legacy RDP, many Windows bitmap updates do NOT include
|
||||
FrameMarker PDUs. guac_display_render_thread_notify_frame() (which wakes the
|
||||
render thread to encode dirty tiles) is ONLY called from guac_rdp_gdi_mark_frame
|
||||
— the FrameMarker handler. Legacy bitmaps that arrive without FrameMarkers mark
|
||||
regions dirty but never wake the render thread → tiles are never encoded →
|
||||
black squares on screen that only appear when a subsequent hover redraw (which
|
||||
DOES carry a FrameMarker) finally wakes the thread for that region.
|
||||
|
||||
Root cause: the gdi_modified → notify_modified path in the main event loop only
|
||||
fires AFTER the RDP event queue drains. During a continuous burst of bitmap
|
||||
updates (initial desktop load), the queue never drains, so notify_modified is
|
||||
never called, and the render thread sleeps indefinitely. Additionally, the render
|
||||
thread applies lag compensation: if the browser hasn't ACK'd the last sync
|
||||
instruction, processing_lag can reach 500 ms, and the render thread pauses up to
|
||||
500 ms between flushes. Both factors combine to cause large areas of the screen
|
||||
to remain black throughout the initial render.
|
||||
|
||||
Fix: call notify_frame unconditionally on every EndPaint. The render thread
|
||||
naturally rate-limits output via MIN_FRAME_DURATION (10 ms = 100 fps max). To
|
||||
prevent the processing_lag from reaching the 500 ms cap (which would cause the
|
||||
render thread to throttle itself), the Node.js proxy (rdp.ts) immediately echoes
|
||||
every sync instruction received from guacd back to guacd as a client ACK. This
|
||||
keeps processing_lag near 0 ms so the render thread flushes as fast as
|
||||
MIN_FRAME_DURATION allows. The browser's own sync ACKs are dropped by the proxy
|
||||
to avoid double-updating the processing_lag measurement.
|
||||
"""
|
||||
|
||||
import sys
|
||||
|
||||
|
||||
def patch_file_replace(path, search_string, replacement_fn, description):
|
||||
"""Replace the line containing search_string with replacement_fn(indent)."""
|
||||
with open(path) as f:
|
||||
lines = f.readlines()
|
||||
|
||||
patched = False
|
||||
i = 0
|
||||
while i < len(lines):
|
||||
if search_string in lines[i]:
|
||||
indent = lines[i][: len(lines[i]) - len(lines[i].lstrip())]
|
||||
lines[i] = indent + replacement_fn(indent) + "\n"
|
||||
patched = True
|
||||
break
|
||||
i += 1
|
||||
|
||||
if not patched:
|
||||
print("ERROR: patch target not found in " + path + " (" + description + ")", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
with open(path, "w") as f:
|
||||
f.writelines(lines)
|
||||
|
||||
print("Patched " + path + " (" + description + ")")
|
||||
|
||||
|
||||
def patch_file_insert_after(path, search_string, insert_lines, description):
|
||||
"""Insert insert_lines (with matching indent) after the line containing search_string."""
|
||||
with open(path) as f:
|
||||
lines = f.readlines()
|
||||
|
||||
patched = False
|
||||
i = 0
|
||||
while i < len(lines):
|
||||
if search_string in lines[i]:
|
||||
indent = lines[i][: len(lines[i]) - len(lines[i].lstrip())]
|
||||
for j, new_line in enumerate(insert_lines):
|
||||
lines.insert(i + 1 + j, indent + new_line + "\n")
|
||||
patched = True
|
||||
break
|
||||
i += 1
|
||||
|
||||
if not patched:
|
||||
print("ERROR: patch target not found in " + path + " (" + description + ")", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
with open(path, "w") as f:
|
||||
f.writelines(lines)
|
||||
|
||||
print("Patched " + path + " (" + description + ")")
|
||||
|
||||
|
||||
# Fix 1: display-flush.c
|
||||
patch_file_replace(
|
||||
"src/libguac/display-flush.c",
|
||||
"GUAC_ASSERT(current->pending_frame.buffer_is_external)",
|
||||
lambda indent: "current = current->pending_frame.next;",
|
||||
"replace GUAC_ASSERT with pointer advance",
|
||||
)
|
||||
|
||||
# Fix 2: display-plan.c (identical bug pattern)
|
||||
patch_file_replace(
|
||||
"src/libguac/display-plan.c",
|
||||
"GUAC_ASSERT(current->pending_frame.buffer_is_external)",
|
||||
lambda indent: "current = current->pending_frame.next;",
|
||||
"replace GUAC_ASSERT with pointer advance",
|
||||
)
|
||||
|
||||
# Fix 3: gdi.c - handle begin_paint context still open during desktop_resize.
|
||||
# With the GFX pipeline on FreeRDP < 3.8.0, begin_paint may be active when
|
||||
# desktop_resize fires, leaving current_context non-NULL. FreeRDP 3.8.0 fixed
|
||||
# this by calling EndPaint inside gdi_resize(); backport that behaviour here.
|
||||
gdi_path = "src/protocols/rdp/gdi.c"
|
||||
with open(gdi_path) as f:
|
||||
gdi_content = f.read()
|
||||
|
||||
gdi_old = (
|
||||
" /* For FreeRDP versions prior to 3.8.0, EndPaint will not be called in\n"
|
||||
" * `gdi_resize()`, so the current context should be NULL. If it is not\n"
|
||||
" * NULL, it means that the current context is still open, and therefore the\n"
|
||||
" * GDI buffer has not been flushed yet. */\n"
|
||||
" GUAC_ASSERT(rdp_client->current_context == NULL);"
|
||||
)
|
||||
|
||||
gdi_new = (
|
||||
" /* For FreeRDP < 3.8.0, gdi_resize() does not call EndPaint internally.\n"
|
||||
" * With the GFX pipeline begin_paint may still be active when desktop_resize\n"
|
||||
" * fires, leaving current_context non-NULL. End the paint now so the context\n"
|
||||
" * is properly closed before we open a new one for the resize. */\n"
|
||||
" if (rdp_client->current_context != NULL)\n"
|
||||
" guac_rdp_gdi_end_paint(context);"
|
||||
)
|
||||
|
||||
if gdi_old not in gdi_content:
|
||||
print("ERROR: patch target not found in " + gdi_path + " (desktop_resize assert)", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
gdi_content = gdi_content.replace(gdi_old, gdi_new, 1)
|
||||
with open(gdi_path, "w") as f:
|
||||
f.write(gdi_content)
|
||||
|
||||
print("Patched " + gdi_path + " (desktop_resize assert -> end_paint guard)")
|
||||
|
||||
|
||||
# Fix 5: user.c - NULL guard for image_mimetypes in guac_user_supports_webp
|
||||
patch_file_insert_after(
|
||||
"src/libguac/user.c",
|
||||
"const char** mimetype = user->info.image_mimetypes;",
|
||||
[
|
||||
"",
|
||||
"/* Guard: image_mimetypes may be NULL if client sent no image types */",
|
||||
"if (mimetype == NULL)",
|
||||
" return 0;",
|
||||
"",
|
||||
],
|
||||
"NULL guard for image_mimetypes in guac_user_supports_webp",
|
||||
)
|
||||
|
||||
# Fix 6: gdi.c - notify_frame on every EndPaint (no rate limit)
|
||||
#
|
||||
# Problem: legacy RDP bitmap-cache updates (taskbar icons, static content) carry
|
||||
# no FrameMarker PDU. The gdi_modified → notify_modified path only fires after
|
||||
# the RDP event queue drains — never during a burst — so tiles accumulate as
|
||||
# black squares. Additionally the render thread's lag-compensation can pause up
|
||||
# to 500 ms between flushes if the browser hasn't ACK'd the previous sync.
|
||||
#
|
||||
# Fix: call notify_frame unconditionally in EndPaint. The render thread's own
|
||||
# MIN_FRAME_DURATION (10 ms) provides natural batching at up to 100 fps. The
|
||||
# sync-flood / lag-compensation problem is solved at the proxy layer (rdp.ts):
|
||||
# the Node.js proxy immediately echoes every sync timestamp back to guacd so
|
||||
# processing_lag stays near 0 ms, and the browser's own ACKs are dropped to
|
||||
# prevent double-updating the measurement.
|
||||
gdi_end_paint_old = (
|
||||
" /* There will be no further drawing operations */\n"
|
||||
" rdp_client->current_context = NULL;\n"
|
||||
" guac_display_layer_close_raw(default_layer, current_context);\n"
|
||||
"\n"
|
||||
" return TRUE;\n"
|
||||
)
|
||||
|
||||
gdi_end_paint_new = (
|
||||
" /* There will be no further drawing operations */\n"
|
||||
" rdp_client->current_context = NULL;\n"
|
||||
" guac_display_layer_close_raw(default_layer, current_context);\n"
|
||||
"\n"
|
||||
" /* Notify the render thread that new tiles are ready to encode.\n"
|
||||
" * notify_frame rather than notify_modified ensures the frame counter\n"
|
||||
" * advances and a sync is emitted after each batch. This causes legacy\n"
|
||||
" * bitmap-cache updates (which carry no FrameMarker PDU) to be flushed\n"
|
||||
" * immediately on every EndPaint instead of accumulating as black squares\n"
|
||||
" * until the next FrameMarker. The render thread's MIN_FRAME_DURATION\n"
|
||||
" * (10 ms) limits output to 100 fps; the Node.js proxy handles sync ACKs\n"
|
||||
" * so guacd's processing_lag stays near 0 ms. */\n"
|
||||
" if (rdp_client->render_thread != NULL)\n"
|
||||
" guac_display_render_thread_notify_frame(rdp_client->render_thread);\n"
|
||||
"\n"
|
||||
" return TRUE;\n"
|
||||
)
|
||||
|
||||
gdi_path = "src/protocols/rdp/gdi.c"
|
||||
with open(gdi_path) as f:
|
||||
gdi_end_content = f.read()
|
||||
|
||||
if gdi_end_paint_old not in gdi_end_content:
|
||||
print("ERROR: patch target not found in " + gdi_path + " (notify_frame on EndPaint)", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
gdi_end_content = gdi_end_content.replace(gdi_end_paint_old, gdi_end_paint_new, 1)
|
||||
with open(gdi_path, "w") as f:
|
||||
f.write(gdi_end_content)
|
||||
|
||||
print("Patched " + gdi_path + " (notify_frame on every EndPaint)")
|
||||
Reference in New Issue
Block a user