From f4194133f9886c9ea2283eb86b759fdde2092e3e Mon Sep 17 00:00:00 2001 From: hujianwei Date: Tue, 8 Nov 2022 03:37:37 +0000 Subject: [PATCH] Better handle X server out-of-memory situations * compositor.h (struct _RenderFuncs): Add function `set_client'. (struct _ClientErrorData): New struct. * icon_surface.c (XLGetIconSurface): Attach client to render target. * picture_renderer.c (struct _BackBuffer): Keep track of how many pixels were allocated. (struct _PictureTarget): Keep track of the client data. (SetClient): New function. (FreeBackBuffer, FreeBackBuffers, CreateBackBuffer) (SetStandardEventMask, NoteTargetSize, DestroyRenderTarget) (picture_render_funcs): Keep track of the number of pixels allocated on behalf of each client. * renderer.c (RenderSetClient): New function. * run.c (RunStep): Disconnect clients pending disconnect. * shm.c (InitRender): Export the render error base. * test.c (GetTestSurface): * xdg_surface.c (XLGetXdgSurface): Attach the client to the render target. * xerror.c (enum _ClientMemoryCategory): New enum. (ReleaseClientData, HandleClientDestroy, ErrorDataForClient) (CategorizeClients, SavePendingDisconnectClient) (DisconnectOneClient, ProcessPendingDisconnectClients) (HandleBadAlloc): New functions. (ErrorHandler): Handle BadAlloc errors by disconnecting pixmap hogs. --- compositor.h | 30 ++++ icon_surface.c | 5 + picture_renderer.c | 109 ++++++++++++++- renderer.c | 9 ++ run.c | 12 ++ shm.c | 7 +- test.c | 3 + xdg_surface.c | 3 + xerror.c | 334 +++++++++++++++++++++++++++++++++++++++++++-- 9 files changed, 497 insertions(+), 15 deletions(-) diff --git a/compositor.h b/compositor.h index 7874e3e..80aa4f3 100644 --- a/compositor.h +++ b/compositor.h @@ -325,6 +325,10 @@ struct _RenderFuncs /* Create a rendering target for the given pixmap. */ RenderTarget (*target_from_pixmap) (Pixmap); + /* Set the client associated with the target. This allows some + extra pixmap memory allocation tracking to be done. */ + void (*set_client) (RenderTarget, struct wl_client *); + /* Set the standard event mask of the target. */ void (*set_standard_event_mask) (RenderTarget, unsigned long); @@ -537,6 +541,7 @@ extern void InitRenderers (void); extern RenderTarget RenderTargetFromWindow (Window, unsigned long); extern RenderTarget RenderTargetFromPixmap (Pixmap); +extern void RenderSetClient (RenderTarget, struct wl_client *); extern void RenderSetStandardEventMask (RenderTarget, unsigned long); extern void RenderNoteTargetSize (RenderTarget, int, int); extern Picture RenderPictureFromTarget (RenderTarget); @@ -742,6 +747,8 @@ extern void ExtBufferDestroy (ExtBuffer *); /* Defined in shm.c. */ +extern int render_first_error; + extern void XLInitShm (void); /* Defined in subcompositor.c. */ @@ -1408,6 +1415,29 @@ extern void XLInitPopups (void); /* Defined in xerror.c. */ +typedef struct _ClientErrorData ClientErrorData; + +struct _ClientErrorData +{ + /* The wl_listener used to hang this data from. */ + struct wl_listener listener; + + /* How many pixels of pixmap data this client has allocated. This + data is guaranteed to be present as long as the client still + exists. */ + uint64_t n_pixels; + + /* The number of references to this error data. The problem with is + that resources are destroyed after the client error data, so + without reference counting the client data will become invalid by + the time the destroy listener is called. */ + int refcount; +}; + +extern ClientErrorData *ErrorDataForClient (struct wl_client *); +extern void ReleaseClientData (ClientErrorData *); +extern void ProcessPendingDisconnectClients (void); + extern void InitXErrors (void); extern void CatchXErrors (void); extern Bool UncatchXErrors (XErrorEvent *); diff --git a/icon_surface.c b/icon_surface.c index bf3570f..035fb4b 100644 --- a/icon_surface.c +++ b/icon_surface.c @@ -416,6 +416,11 @@ XLGetIconSurface (Surface *surface) /* Create a target associated with the window. */ role->target = RenderTargetFromWindow (role->window, None); + /* Set the client. */ + if (surface->resource) + RenderSetClient (role->target, + wl_resource_get_client (surface->resource)); + /* For simplicity reasons we do not handle idle notifications asynchronously. */ RenderSetNeedWaitForIdle (role->target); diff --git a/picture_renderer.c b/picture_renderer.c index 1e0f6d4..8bacfdf 100644 --- a/picture_renderer.c +++ b/picture_renderer.c @@ -183,6 +183,9 @@ struct _PresentCompletionCallback struct _BackBuffer { + /* How many pixels were allocated. */ + uint64_t n_pixels; + /* The picture of this back buffer. High bit means the back buffer is busy. */ Picture picture; @@ -228,6 +231,10 @@ struct _PictureTarget /* Two back buffers. */ BackBuffer *back_buffers[2]; + /* Structure used to allocate the amount of pixmap allocated on + behalf of a client. */ + ClientErrorData *client; + /* Presentation event context. */ XID presentation_event_context; @@ -699,7 +706,7 @@ HandleActivityEvent (uint64_t counter) static void -FreeBackBuffer (BackBuffer *buffer) +FreeBackBuffer (PictureTarget *target, BackBuffer *buffer) { XRenderFreePicture (compositor.display, (buffer->picture & ~BufferSync @@ -707,6 +714,15 @@ FreeBackBuffer (BackBuffer *buffer) XFreePixmap (compositor.display, (buffer->pixmap & ~BufferSync)); FenceRelease (buffer->idle_fence); + + /* Subtract the amount of pixels allocated from the target. */ + if (target->client + && IntSubtractWrapv (target->client->n_pixels, + buffer->n_pixels, + &target->client->n_pixels)) + /* Handle overflow by just setting n_pixels to 0. */ + target->client->n_pixels = 0; + XLFree (buffer); } @@ -718,7 +734,7 @@ FreeBackBuffers (PictureTarget *target) for (i = 0; i < ArrayElements (target->back_buffers); ++i) { if (target->back_buffers[i]) - FreeBackBuffer (target->back_buffers[i]); + FreeBackBuffer (target, target->back_buffers[i]); } /* Also clear target->picture if it is a window target. */ @@ -753,6 +769,18 @@ CreateBackBuffer (PictureTarget *target) /* The target is no longer freshly presented. */ target->flags &= ~JustPresented; + /* Calculate how many pixels would be allocated and add it to the + target data. */ + + if (IntMultiplyWrapv (target->width, target->height, + &buffer->n_pixels)) + buffer->n_pixels = UINT64_MAX; + + if (target->client + && IntAddWrapv (target->client->n_pixels, buffer->n_pixels, + &target->client->n_pixels)) + target->client->n_pixels = UINT64_MAX; + return buffer; } @@ -1253,6 +1281,40 @@ TargetFromWindow (Window window, unsigned long event_mask) return TargetFromDrawable (window, window, event_mask); } +static void +SetClient (RenderTarget target, struct wl_client *client) +{ + PictureTarget *picture_target; + ClientErrorData *data; + uint64_t pixels; + + picture_target = target.pointer; + + /* Release the client data if some is already attached. */ + if (picture_target->client) + { + if (picture_target->client + && IntMultiplyWrapv (picture_target->width, + picture_target->height, + &pixels) + && IntSubtractWrapv (picture_target->client->n_pixels, + pixels, + &picture_target->client->n_pixels)) + picture_target->client->n_pixels = 0; + + ReleaseClientData (picture_target->client); + } + picture_target->client = NULL; + + if (!client) + return; + + /* Retain the client data. */ + data = ErrorDataForClient (client); + picture_target->client = data; + data->refcount++; +} + static void SetStandardEventMask (RenderTarget target, unsigned long standard_event_mask) { @@ -1269,13 +1331,34 @@ static void NoteTargetSize (RenderTarget target, int width, int height) { PictureTarget *pict_target; + uint64_t pixels; pict_target = target.pointer; if (width != pict_target->width || height != pict_target->height) - /* Recreate all the back buffers for the new target size. */ - FreeBackBuffers (pict_target); + { + /* Recreate all the back buffers for the new target size. */ + FreeBackBuffers (pict_target); + + /* First, remove existing pixels from the client. */ + if (pict_target->client + && IntMultiplyWrapv (pict_target->width, + pict_target->height, + &pixels) + && IntSubtractWrapv (pict_target->client->n_pixels, + pixels, + &pict_target->client->n_pixels)) + pict_target->client->n_pixels = 0; + + /* Next, add the new width and height to the client. */ + if (pict_target->client + && IntMultiplyWrapv (width, height, &pixels) + && IntAddWrapv (pict_target->client->n_pixels, + pixels, + &pict_target->client->n_pixels)) + pict_target->client->n_pixels = UINT64_MAX; + } pict_target->width = width; pict_target->height = height; @@ -1314,6 +1397,7 @@ DestroyRenderTarget (RenderTarget target) PresentRecord *record, *last; BufferActivityRecord *activity_record, *activity_last; IdleCallback *idle, *idle_last; + uint64_t pixels; pict_target = target.pointer; @@ -1370,6 +1454,22 @@ DestroyRenderTarget (RenderTarget target) XRenderFreePicture (compositor.display, pict_target->picture); + /* Dereference the client data if it is set. Also, remove the + pixels recorded. */ + if (pict_target->client) + { + if (pict_target->client + && IntMultiplyWrapv (pict_target->width, + pict_target->height, + &pixels) + && IntSubtractWrapv (pict_target->client->n_pixels, + pixels, + &pict_target->client->n_pixels)) + pict_target->client->n_pixels = 0; + + ReleaseClientData (pict_target->client); + } + XFree (pict_target); } @@ -1899,6 +1999,7 @@ static RenderFuncs picture_render_funcs = .init_render_funcs = InitRenderFuncs, .target_from_window = TargetFromWindow, .target_from_pixmap = TargetFromPixmap, + .set_client = SetClient, .set_standard_event_mask = SetStandardEventMask, .note_target_size = NoteTargetSize, .picture_from_target = PictureFromTarget, diff --git a/renderer.c b/renderer.c index e98c874..3ccf559 100644 --- a/renderer.c +++ b/renderer.c @@ -80,6 +80,15 @@ RenderTargetFromPixmap (Pixmap pixmap) return render_funcs.target_from_pixmap (pixmap); } +void +RenderSetClient (RenderTarget target, struct wl_client *client) +{ + if (!render_funcs.set_client) + return; + + render_funcs.set_client (target, client); +} + void RenderSetStandardEventMask (RenderTarget target, unsigned long standard_event_mask) diff --git a/run.c b/run.c index 6b01403..574d91c 100644 --- a/run.c +++ b/run.c @@ -215,6 +215,10 @@ RunStep (void) /* Drain complete selection transfers. */ FinishTransfers (); + /* Disconnect clients that have experienced out-of-memory + errors. */ + ProcessPendingDisconnectClients (); + /* FinishTransfers can potentially send events to Wayland clients and make X requests. Flush after it is called. */ XFlush (compositor.display); @@ -273,6 +277,10 @@ RunStep (void) wl_display_flush_clients (compositor.wl_display); } + /* Disconnect clients that have experienced out-of-memory + errors. */ + ProcessPendingDisconnectClients (); + rc = ProcessPoll (fds, 2 + i, &timeout); if (rc > 0) @@ -295,6 +303,10 @@ RunStep (void) pollfds[j]->data, pollfds[j]); } } + + /* Disconnect clients that have experienced out-of-memory + errors. */ + ProcessPendingDisconnectClients (); } void __attribute__ ((noreturn)) diff --git a/shm.c b/shm.c index d548f69..746ac79 100644 --- a/shm.c +++ b/shm.c @@ -77,6 +77,9 @@ typedef struct _Buffer /* The shared memory global. */ static struct wl_global *global_shm; +/* The error base of the Render extension. */ +int render_first_error; + static void DereferencePool (Pool *pool) { @@ -534,10 +537,10 @@ HandleBind (struct wl_client *client, void *data, static void InitRender (void) { - int major, minor, base, dummy; + int major, minor, base; if (!XRenderQueryExtension (compositor.display, - &base, &dummy)) + &base, &render_first_error)) { fprintf (stderr, "XRender is not supported by this X server\n"); exit (1); diff --git a/test.c b/test.c index 43f2c5f..0dc33dd 100644 --- a/test.c +++ b/test.c @@ -457,6 +457,9 @@ GetTestSurface (struct wl_client *client, struct wl_resource *resource, test->subcompositor = MakeSubcompositor (); test->target = RenderTargetFromWindow (test->window, DefaultEventMask); + /* Set the client. */ + RenderSetClient (test->target, client); + /* And a buffer release helper. */ test->release_helper = MakeBufferReleaseHelper (AllBuffersReleased, test); diff --git a/xdg_surface.c b/xdg_surface.c index e7812c9..7717778 100644 --- a/xdg_surface.c +++ b/xdg_surface.c @@ -1475,6 +1475,9 @@ XLGetXdgSurface (struct wl_client *client, struct wl_resource *resource, role->release_helper = MakeBufferReleaseHelper (AllBuffersReleased, role); + /* Set the client. */ + RenderSetClient (role->target, client); + role->subcompositor = MakeSubcompositor (); role->clock = XLMakeFrameClockForWindow (role->window); XLFrameClockSetFreezeCallback (role->clock, HandleFreeze, role); diff --git a/xerror.c b/xerror.c index 6d8e20b..99b91e0 100644 --- a/xerror.c +++ b/xerror.c @@ -24,6 +24,19 @@ along with 12to11. If not, see . */ #include +typedef enum _ClientMemoryCategory ClientMemoryCategory; + +/* See the comment in HandleBadAlloc for the meaning of these + enumerators. */ +enum _ClientMemoryCategory + { + MemoryCategoryI, + MemoryCategoryII, + MemoryCategoryIII, + MemoryCategoryIV, + MemoryCategoryV, + }; + /* X error handling routines. The entry point into this code is CatchXErrors, which starts catching X errors in the following code, and UncatchXErrors, which syncs (if necessary) and saves any @@ -34,18 +47,28 @@ along with 12to11. If not, see . */ /* First request from which errors should be caught. -1 if we are not currently catching errors. */ - -static unsigned long first_error_req; +static long long first_error_req; /* XErrorEvent any error that happens while errors are being caught will be saved in. */ - static XErrorEvent error; /* True if any error was caught. */ - static Bool error_caught; +/* True if any BadAlloc error was encountered. */ +static Bool bad_alloc_experienced; + +/* The serial of the last BadAlloc request. */ +static unsigned long next_bad_alloc_serial; + +/* Whether or not program execution is currently inside the bad alloc + handler. */ +static Bool inside_bad_alloc_handler; + +/* Clients that are pending disconnect. */ +static XLList *pending_disconnect_clients; + void CatchXErrors (void) { @@ -78,10 +101,290 @@ UncatchXErrors (XErrorEvent *event) return True; } +void +ReleaseClientData (ClientErrorData *data) +{ + if (--data->refcount) + return; + + XLFree (data); +} + +static void +HandleClientDestroy (struct wl_listener *listener, void *data) +{ + struct wl_client *client; + + /* The client has been destroyed. Remove it from the list of + clients pending destruction. */ + client = data; + pending_disconnect_clients + = XLListRemove (pending_disconnect_clients, client); + + /* listener is actually the ClientErrorData. */ + ReleaseClientData ((ClientErrorData *) listener); +} + +ClientErrorData * +ErrorDataForClient (struct wl_client *client) +{ + struct wl_listener *listener; + ClientErrorData *data; + + listener = wl_client_get_destroy_listener (client, + HandleClientDestroy); + + if (listener) + return (ClientErrorData *) listener; + + /* Allocate the data and set it as the client's destroy + listener. */ + + data = XLMalloc (sizeof *data); + data->listener.notify = HandleClientDestroy; + data->n_pixels = 0; + + /* Add a reference to the data. */ + data->refcount = 1; + + wl_client_add_destroy_listener (client, &data->listener); + return data; +} + +static void +CategorizeClients (struct wl_list *client_list, + struct wl_client **clients, + ClientMemoryCategory *categories, + ClientMemoryCategory *max_category) +{ + struct wl_list *list; + struct wl_client *client; + ClientErrorData *data; + uint64_t total; + int i; + + total = 0; + i = 0; + list = client_list; + + /* The heuristics here are not intended to handle every out of + memory situation. Rather, they are designed to handle BadAlloc + errors caused by clients recently allocating unreasonable chunks + of memory from the server. */ + + /* Compute the total number of pixels allocated. */ + + wl_client_for_each (client, list) + { + data = ErrorDataForClient (client); + + if (IntAddWrapv (total, data->n_pixels, &total)) + total = UINT64_MAX; + } + + list = client_list; + + wl_client_for_each (client, list) + { + data = ErrorDataForClient (client); + + if (data->n_pixels <= total * 0.05) + { + if (*max_category < MemoryCategoryI) + *max_category = MemoryCategoryI; + + categories[i++] = MemoryCategoryI; + clients[i - 1] = client; + } + else if (data->n_pixels <= total * 0.10) + { + if (*max_category < MemoryCategoryII) + *max_category = MemoryCategoryII; + + categories[i++] = MemoryCategoryII; + clients[i - 1] = client; + } + else if (data->n_pixels <= total * 0.20) + { + if (*max_category < MemoryCategoryIII) + *max_category = MemoryCategoryIII; + + categories[i++] = MemoryCategoryIII; + clients[i - 1] = client; + } + else if (data->n_pixels <= total / 2) + { + if (*max_category < MemoryCategoryIV) + *max_category = MemoryCategoryIV; + + categories[i++] = MemoryCategoryIV; + clients[i - 1] = client; + } + else + { + if (*max_category < MemoryCategoryV) + *max_category = MemoryCategoryV; + + categories[i++] = MemoryCategoryV; + clients[i - 1] = client; + } + + fprintf (stderr, + "%p %"PRIu64" %"PRIu64" %u\n", client, total, data->n_pixels, + categories[i - 1]); + } +} + +static void +SavePendingDisconnectClient (struct wl_client *client) +{ + XLList *list; + + /* Never link the same client onto the list twice. */ + + list = pending_disconnect_clients; + for (; list; list = list->next) + { + if (list->data == client) + return; + } + + pending_disconnect_clients + = XLListPrepend (pending_disconnect_clients, client); +} + +static void +DisconnectOneClient (void *data) +{ + wl_client_post_no_memory (data); +} + +void +ProcessPendingDisconnectClients (void) +{ + XLList *head; + + /* Unlink the list onto head. */ + head = pending_disconnect_clients; + pending_disconnect_clients = NULL; + + if (head) + XLListFree (head, DisconnectOneClient); + + /* Ignore future BadAlloc errors caused by requests generated before + client disconnects were processed. */ + next_bad_alloc_serial + = XNextRequest (compositor.display); +} + +static void +HandleBadAlloc (XErrorEvent *event) +{ + struct wl_client **clients_by_category; + struct wl_list *client_list; + int num_clients, i; + char buf[256]; + ClientMemoryCategory *categories, max_category; + + if (event->serial < next_bad_alloc_serial) + return; + + if (inside_bad_alloc_handler) + /* This function is not reentrant. */ + return; + + /* Handle a BadAlloc error. For efficiency reasons, errors are not + caught around events (CreatePixmap, CreateWindow, etc) that are + likely to raise BadAlloc errors upon ridiculous requests from + clients do not have errors caught around them. + + Upon such an error occuring, clients are sorted by the amount of + pixmap they have allocated. Each client has its own "badness" + score for each pixel of pixmap data allocated on its behalf. + + Each client is organized by the percentage of the total badness + it takes up. Those clients are then categorized as follows: + + I. Clients occupying between 0 to 5 percent of the total + score. + II. Clients occupying between 6 to 10 percent of the total + score. + III. Clients occupying between 11 to 20 percent of the total + score. + IV. Clients occupying more than 20 percent of the total score. + V. Clients occupying more than 50 percent of the total score. + + Finally, all clients falling in the bottom-most category that + exists are disconnected with out-of-memory errors. */ + + client_list = wl_display_get_client_list (compositor.wl_display); + + if (wl_list_empty (client_list)) + { + /* If there are no clients, just exit immediately. */ + XGetErrorText (compositor.display, event->error_code, + buf, sizeof buf); + fprintf (stderr, "X protocol error: %s on protocol request %d\n", + buf, event->request_code); + exit (70); + } + + /* Count the number of clients. */ + num_clients = wl_list_length (client_list); + + /* Allocate buffers to hold the client data. */ + categories + = alloca (sizeof *categories * num_clients); + clients_by_category + = alloca (sizeof *clients_by_category * num_clients); + max_category + = MemoryCategoryI; + + /* Organize the clients by category. */ + CategorizeClients (client_list, clients_by_category, + categories, &max_category); + + /* Ignore future BadAlloc errors caused by requests generated before + this BadAlloc error was processed. */ + next_bad_alloc_serial = XNextRequest (compositor.display); + + /* Prevent recursive invocations of this error handler when XSync is + called by resource destructors. */ + inside_bad_alloc_handler = True; + + /* Now disconnect each client fitting that category. + wl_client_post_no_memory can call Xlib functions through resource + destructors, so each client is actually put on a queue that is + drained in RunStep. */ + for (i = 0; i < num_clients; ++i) + { + if (categories[i] == max_category) + SavePendingDisconnectClient (clients_by_category[i]); + } + + /* At this point, start ignoring all BadDrawable, BadWindow, + BadPixmap and BadPicture errors. Whatever resource was supposed + to be created could not be created, so trying to destroy it as + part of client destruction will fail. */ + bad_alloc_experienced = True; + inside_bad_alloc_handler = False; +} + static int ErrorHandler (Display *display, XErrorEvent *event) { char buf[256]; + unsigned long next_request; + + /* Reset fields that overflowed. */ + next_request = XNextRequest (compositor.display); + + if (first_error_req != -1 + && next_request < first_error_req) + first_error_req = 0; + + if (next_request < next_bad_alloc_serial) + next_bad_alloc_serial = 0; if (first_error_req != -1 && event->serial >= first_error_req) @@ -92,11 +395,6 @@ ErrorHandler (Display *display, XErrorEvent *event) return 0; } -#if 0 - if (XLHandleErrorForDmabuf (event)) - return 0; -#endif - if (HandleErrorForPictureRenderer (event)) return 0; @@ -106,6 +404,24 @@ ErrorHandler (Display *display, XErrorEvent *event) processed the corresponding hierarchy events. */ return 0; + if (bad_alloc_experienced + /* See the comment at the end of HandleBadAlloc for why this is + done. */ + && (event->error_code == BadDrawable + || event->error_code == BadWindow + || event->error_code == BadPixmap + || event->error_code == (render_first_error + BadPicture))) + return 0; + + if (event->error_code == BadAlloc) + { + /* A client may have asked for the protocol translator to + allocate an unreasonable amount of memory. */ + HandleBadAlloc (event); + + return 0; + } + XGetErrorText (display, event->error_code, buf, sizeof buf); fprintf (stderr, "X protocol error: %s on protocol request %d\n", buf, event->request_code);