Re: [PATCH v2 wayland 00/11] Stop leaking file descriptors
Hi Derek, On 13 April 2017 at 17:51, Derek Foreman wrote: > Moved the test cases to the end so they're not introduced in a failed > state. > > Reworked the removal of the global zombie singleton patch - we now > create a wl_zombie at proxy creation time and store the number of > fds for each opcode in that. If no signature requires fds we just > use a NULL pointer to avoid useless malloc/free. At client > disconnect we do a map walk to inter any zombie left behind and > make sure we don't leak memory. > > Thanks to Pekka and Jonas for suggestions on how to handle this > without breaking API as I'd previously done. > > To perform the map walk I added a new patch that changes the > wl_map_for_each() function to provide the entry flags - we can't > look up flags from the iterator callback without dereferencing the > pointer, and with my changes we don't know what the data type stored > in that map location is. This only changes internal callers - the > publicly visible map walk function still acts as before. Thanks for this. It looks fairly good to me so far. I've pushed 1, 5 and 9 as trivially correct. As said in comments to 4, I feel like a slightly improved 4 could also replace 2 and 3 at the same time. 6, 8 and 10 all have my R-b, though a comment as to why three is the correct number of roundtrips wouldn't go astray on 10 (it seems right to me, mind). Unfortunately I'm not getting far into 11 without my eyes glazing over, and I haven't given 7 a pass over for actual lifetime handling yet, but that at least gets my A-b. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2 wayland 00/11] Stop leaking file descriptors
Changes since the first edition: Moved the test cases to the end so they're not introduced in a failed state. Reworked the removal of the global zombie singleton patch - we now create a wl_zombie at proxy creation time and store the number of fds for each opcode in that. If no signature requires fds we just use a NULL pointer to avoid useless malloc/free. At client disconnect we do a map walk to inter any zombie left behind and make sure we don't leak memory. Thanks to Pekka and Jonas for suggestions on how to handle this without breaking API as I'd previously done. To perform the map walk I added a new patch that changes the wl_map_for_each() function to provide the entry flags - we can't look up flags from the iterator callback without dereferencing the pointer, and with my changes we don't know what the data type stored in that map location is. This only changes internal callers - the publicly visible map walk function still acts as before. And I just realized I included a completely unrelated trivial refactor. Thanks, Derek Derek Foreman (11): connection: close_fds() should only remove fds it closed from the buffer connection: Close fds from half marshalled closures connection: Close fds from half demarshalled closures connection: Make wl_closure_destroy() close fds of undispatched closures client: Simplify some logic in queue_event util: Pass flags to map iterators client: Replace the singleton zombie with bespoke zombies client: Consume file descriptors destined for zombie proxies connection: Use wl_buffer_size() for all buffer size calculations tests: Add a test for fd leaks on zombie objects tests: Check for wrong fd delivery with zombie objects Makefile.am | 7 +- protocol/tests.xml | 52 +++ src/connection.c | 75 +-- src/wayland-client.c | 143 +++- src/wayland-private.h| 17 ++-- src/wayland-server.c | 27 -- src/wayland-util.c | 4 +- tests/connection-test.c | 12 +-- tests/display-test.c | 239 +++ tests/os-wrappers-test.c | 4 +- 10 files changed, 526 insertions(+), 54 deletions(-) create mode 100644 protocol/tests.xml -- 2.11.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
