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.

Sign up for GitHub

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

Jump to bottom

Avoid defragging scripts during eval #1414

Merged
merged 2 commits into from
Dec 12, 2024
Merged

Avoid defragging scripts during eval #1414

merged 2 commits into from
Dec 12, 2024

Conversation

ranshid
Copy link
Member

@ranshid ranshid commented Dec 10, 2024

This can happen when long scripts are being run and we are attempting to defrag it in the whileBlockedCron.

Example:

Logged sanitizer errors (pid 13967):
=================================================================
==13967==ERROR: AddressSanitizer: heap-use-after-free on address 0x60300004b110 at pc 0x000102d66970 bp 0x00016d27a990 sp 0x00016d27a988
READ of size 8 at 0x60300004b110 thread T0
    #0 0x102d6696c in evalGenericCommand+0xd80 (valkey-server:arm64+0x1001e296c)
    #1 0x102be14e4 in call+0x3cc (valkey-server:arm64+0x10005d4e4)
    #2 0x102be4788 in processCommand+0x17b4 (valkey-server:arm64+0x100060788)
    #3 0x102c1c34c in processInputBuffer+0x498 (valkey-server:arm64+0x10009834c)
    #4 0x102c1a1f8 in readQueryFromClient+0xd4 (valkey-server:arm64+0x1000961f8)
    #5 0x102e60688 in connSocketEventHandler+0x160 (valkey-server:arm64+0x1002dc688)
    #6 0x102bb8f9c in aeProcessEvents+0x35c (valkey-server:arm64+0x100034f9c)
    #7 0x102bfb340 in main+0x6d84 (valkey-server:arm64+0x100077340)
    #8 0x19d1ff150 in start+0x9a8 (dyld:arm64e+0xfffffffffff4d150)

0x60300004b110 is located 16 bytes inside of 24-byte region [0x60300004b100,0x60300004b118)
freed by thread T0 here:
    #0 0x103b30d40 in free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54d40)
    #1 0x102e0a4b8 in activeDefragLuaScript+0x100 (valkey-server:arm64+0x1002864b8)
    #2 0x102bc3730 in dictDefragBucket+0x17c (valkey-server:arm64+0x10003f730)
    #3 0x102e7ddb8 in dictScanDefrag.27435+0x1d4 (valkey-server:arm64+0x1002f9db8)
    #4 0x102e0a2ec in defragLuaScripts+0x150 (valkey-server:arm64+0x1002862ec)
    #5 0x102e08d64 in activeDefragTimeProc+0x2e0 (valkey-server:arm64+0x100284d64)
    #6 0x102bd5648 in whileBlockedCron+0x1b4 (valkey-server:arm64+0x100051648)
    #7 0x102c3b3bc in processEventsWhileBlocked+0x174 (valkey-server:arm64+0x1000b73bc)
    #8 0x102e7059c in luaMaskCountHook+0x1a8 (valkey-server:arm64+0x1002ec59c)
    #9 0x102b937f0 in luaD_callhook+0xcc (valkey-server:arm64+0x10000f7f0)
    #10 0x102b9f840 in luaV_execute+0xb4 (valkey-server:arm64+0x10001b840)
    #11 0x102b94368 in luaD_call+0x74 (valkey-server:arm64+0x100010368)
    #12 0x102b93568 in luaD_rawrunprotected+0x48 (valkey-server:arm64+0x10000f568)
    #13 0x102b94688 in luaD_pcall+0x38 (valkey-server:arm64+0x100010688)
    #14 0x102b8ee94 in lua_pcall+0x128 (valkey-server:arm64+0x10000ae94)
    #15 0x102e6ff6c in luaCallFunction+0x5a4 (valkey-server:arm64+0x1002ebf6c)
    #16 0x102d663c0 in evalGenericCommand+0x7d4 (valkey-server:arm64+0x1001e23c0)
    #17 0x102be14e4 in call+0x3cc (valkey-server:arm64+0x10005d4e4)
    #18 0x102be4788 in processCommand+0x17b4 (valkey-server:arm64+0x100060788)
    #19 0x102c1c34c in processInputBuffer+0x498 (valkey-server:arm64+0x10009834c)
    #20 0x102c1a1f8 in readQueryFromClient+0xd4 (valkey-server:arm64+0x1000961f8)
    #21 0x102e60688 in connSocketEventHandler+0x160 (valkey-server:arm64+0x1002dc688)
    #22 0x102bb8f9c in aeProcessEvents+0x35c (valkey-server:arm64+0x100034f9c)
    #23 0x102bfb340 in main+0x6d84 (valkey-server:arm64+0x100077340)
    #24 0x19d1ff150 in start+0x9a8 (dyld:arm64e+0xfffffffffff4d150)

previously allocated by thread T0 here:
    #0 0x103b30c04 in malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x54c04)
    #1 0x102c09b68 in valkey_malloc+0x38 (valkey-server:arm64+0x100085b68)
    #2 0x102e0a474 in activeDefragLuaScript+0xbc (valkey-server:arm64+0x100286474)
    #3 0x102bc3730 in dictDefragBucket+0x17c (valkey-server:arm64+0x10003f730)
    #4 0x102e7ddb8 in dictScanDefrag.27435+0x1d4 (valkey-server:arm64+0x1002f9db8)
    #5 0x102e0a2ec in defragLuaScripts+0x150 (valkey-server:arm64+0x1002862ec)
    #6 0x102e08d64 in activeDefragTimeProc+0x2e0 (valkey-server:arm64+0x100284d64)
    #7 0x102bb9270 in aeProcessEvents+0x630 (valkey-server:arm64+0x100035270)
    #8 0x102bfb340 in main+0x6d84 (valkey-server:arm64+0x100077340)
    #9 0x19d1ff150 in start+0x9a8 (dyld:arm64e+0xfffffffffff4d150)

@ranshid
Avoid defragging scripts during eval
d0ac884 
This can happen when long scripts are being run and we are attempting to defrag it
in the whileBlockedCron.

Signed-off-by: Ran Shidlansik <[email protected]>
@ranshid ranshid requested a review from madolson December 10, 2024 07:51
@ranshid
Copy link
Member Author

ranshid commented Dec 10, 2024

@JimB123 this one as well - take a look and tell me what you think.

@codecov Codecov
Copy link

codecov bot commented Dec 10, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.84%. Comparing base (e8078b7) to head (30464c2).
Report is 17 commits behind head on unstable.

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable    #1414      +/-   ##
============================================
+ Coverage     70.78%   70.84%   +0.06%     
============================================
  Files           118      118              
  Lines         63561    63613      +52     
============================================
+ Hits          44994    45069      +75     
+ Misses        18567    18544      -23
Files with missing lines Coverage Δ
src/defrag.c 89.61% <100.00%> (+0.01%) ⬆️

... and 18 files with indirect coverage changes

JimB123
JimB123 reviewed Dec 10, 2024
View reviewed changes
@ranshid ranshid requested a review from JimB123 December 10, 2024 17:24
JimB123
JimB123 approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@JimB123 JimB123 left a comment

Choose a reason for hiding this comment

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

Looks good.

hpatro
hpatro approved these changes Dec 12, 2024
View reviewed changes
Copy link
Collaborator

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

LGTM

@hpatro hpatro merged commit 2d92404 into valkey-io:unstable Dec 12, 2024
48 checks passed
vudiep411 pushed a commit to Autxmaton/valkey that referenced this pull request Dec 15, 2024
@ranshid @vudiep411
Avoid defragging scripts during EVAL command execution (valkey-io#1414)
8f18f8a 
This can happen when scripts are running for long period of time and the server attempts to defrag it in the whileBlockedCron.

Signed-off-by: Ran Shidlansik <[email protected]>
kronwerk pushed a commit to kronwerk/valkey that referenced this pull request Jan 27, 2025
@ranshid @kronwerk
Avoid defragging scripts during EVAL command execution (valkey-io#1414)
f5793d8 
This can happen when scripts are running for long period of time and the server attempts to defrag it in the whileBlockedCron.

Signed-off-by: Ran Shidlansik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JimB123 JimB123 JimB123 approved these changes

@hpatro hpatro hpatro approved these changes

@madolson madolson Awaiting requested review from madolson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ranshid @JimB123 @hpatro