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

uv/append: Don't call uvMaybeFireCloseCb in uvAliveSegmentWriterCloseCb #45

Merged

Conversation

freeekanayaka
Copy link
Member

We actually don't track that there are pending segment writer close operations, so this call to uvMaybeFireCloseCb() can't really unblock anything waiting on it.

On the contrary, if it fires after the close callback has actually been fired, it causes a double free:

==199067==ERROR: AddressSanitizer: attempting double-free on 0x602000000010 in thread T0:
#0 0x7fd5e44d7288 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7fd5e4b5a259 in ioCloseCb src/raft.c:115
#2 0x7fd5e4b84861 in uvMaybeFireCloseCbReal src/uv.c:202
#3 0x7fd5e4afba70 in uv_run (/lib/x86_64-linux-gnu/libuv.so.1+0xfa70) (BuildId: 7f7f8c148150666c7b116bf98bf6e27f96c697a9)
#4 0x55b25438ebe5 in SubmitRun tools/benchmark/submit.c:314
#5 0x55b254388955 in main tools/benchmark/main.c:60
#6 0x7fd5e42456c9 (/lib/x86_64-linux-gnu/libc.so.6+0x276c9) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)
#7 0x7fd5e4245784 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27784) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)
#8 0x55b254388bf0 in _start (/home/free/src/c/raft/tools/.libs/raft-benchmark+0x7bf0) (BuildId: 09105ce7d1a7fe71ca812de2af648a85fc490ebb)

0x602000000010 is located 0 bytes inside of 15-byte region [0x602000000010,0x60200000001f) freed by thread T0 here:
#0 0x7fd5e44d7288 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
#1 0x7fd5e4b5a259 in ioCloseCb src/raft.c:115
#2 0x7fd5e4b84861 in uvMaybeFireCloseCbReal src/uv.c:202
#3 0x7fd5e4afba70 in uv_run (/lib/x86_64-linux-gnu/libuv.so.1+0xfa70) (BuildId: 7f7f8c148150666c7b116bf98bf6e27f96c697a9)

previously allocated by thread T0 here:
#0 0x7fd5e44d85bf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7fd5e4b59983 in raft_init src/raft.c:62
#2 0x55b25438e7d4 in serverInit tools/benchmark/submit.c:127
#3 0x55b25438ebbf in SubmitRun tools/benchmark/submit.c:302
#4 0x55b254388955 in main tools/benchmark/main.c:60
#5 0x7fd5e42456c9 (/lib/x86_64-linux-gnu/libc.so.6+0x276c9) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)

…seCb

We actually don't track that there are pending segment writer close operations,
so this call to uvMaybeFireCloseCb() can't really unblock anything waiting on
it.

On the contrary, if it fires after the close callback has actually been fired,
it causes a double free:

=================================================================
==199067==ERROR: AddressSanitizer: attempting double-free on 0x602000000010 in thread T0:
    #0 0x7fd5e44d7288 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    cowsql#1 0x7fd5e4b5a259 in ioCloseCb src/raft.c:115
    cowsql#2 0x7fd5e4b84861 in uvMaybeFireCloseCbReal src/uv.c:202
    cowsql#3 0x7fd5e4afba70 in uv_run (/lib/x86_64-linux-gnu/libuv.so.1+0xfa70) (BuildId: 7f7f8c148150666c7b116bf98bf6e27f96c697a9)
    cowsql#4 0x55b25438ebe5 in SubmitRun tools/benchmark/submit.c:314
    cowsql#5 0x55b254388955 in main tools/benchmark/main.c:60
    cowsql#6 0x7fd5e42456c9  (/lib/x86_64-linux-gnu/libc.so.6+0x276c9) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)
    cowsql#7 0x7fd5e4245784 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27784) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)
    cowsql#8 0x55b254388bf0 in _start (/home/free/src/c/raft/tools/.libs/raft-benchmark+0x7bf0) (BuildId: 09105ce7d1a7fe71ca812de2af648a85fc490ebb)

0x602000000010 is located 0 bytes inside of 15-byte region [0x602000000010,0x60200000001f)
freed by thread T0 here:
    #0 0x7fd5e44d7288 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    cowsql#1 0x7fd5e4b5a259 in ioCloseCb src/raft.c:115
    cowsql#2 0x7fd5e4b84861 in uvMaybeFireCloseCbReal src/uv.c:202
    cowsql#3 0x7fd5e4afba70 in uv_run (/lib/x86_64-linux-gnu/libuv.so.1+0xfa70) (BuildId: 7f7f8c148150666c7b116bf98bf6e27f96c697a9)

previously allocated by thread T0 here:
    #0 0x7fd5e44d85bf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    cowsql#1 0x7fd5e4b59983 in raft_init src/raft.c:62
    cowsql#2 0x55b25438e7d4 in serverInit tools/benchmark/submit.c:127
    cowsql#3 0x55b25438ebbf in SubmitRun tools/benchmark/submit.c:302
    cowsql#4 0x55b254388955 in main tools/benchmark/main.c:60
    cowsql#5 0x7fd5e42456c9  (/lib/x86_64-linux-gnu/libc.so.6+0x276c9) (BuildId: 072feb34c63e054d60d94cbc68d92e4caad25d72)

Signed-off-by: Free Ekanayaka <[email protected]>
@freeekanayaka freeekanayaka merged commit 12b58f6 into cowsql:main Sep 14, 2023
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (a269646) 75.14% compared to head (250dd08) 75.13%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
- Coverage   75.14%   75.13%   -0.01%     
==========================================
  Files          50       50              
  Lines        8931     8929       -2     
  Branches     2214     2214              
==========================================
- Hits         6711     6709       -2     
  Misses        986      986              
  Partials     1234     1234              
Files Changed Coverage Δ
src/uv_append.c 82.37% <ø> (-0.08%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@freeekanayaka freeekanayaka deleted the fix-double-call-to-uv-close-cb branch September 14, 2023 11:50
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