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

MemoryConsumer::try_grow Underflow #4328

Closed
tustvold opened this issue Nov 22, 2022 · 2 comments
Closed

MemoryConsumer::try_grow Underflow #4328

tustvold opened this issue Nov 22, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@tustvold
Copy link
Contributor

Describe the bug

MemoryConsumer::try_grow calls through to MemoryManager::can_grow_directly which determines the remaining space by performing self.pool_size - trk_total.

This can potentially underflow if the tracked memory exceeds the pool size, something for which there is no protection.

To Reproduce

#[test]
fn test_underflow() {
    let config = MemoryManagerConfig::try_new_limit(100, 0.5).unwrap();
    let manager = MemoryManager::new(config);
    manager.grow_tracker_usage(100);

    manager.register_requester(&MemoryConsumerId::new(1));
    assert!(!manager.can_grow_directly(20, 0));
}

Expected behavior

Additional context

@tustvold tustvold added the bug Something isn't working label Nov 22, 2022
@askoa
Copy link
Contributor

askoa commented Nov 23, 2022

I can pick this up. I am planning to return zero if trk_total is more than self.pool_size in below function. Let me know what you think.

https://github.com/apache/arrow-datafusion/blob/df8aa7a2e2a6f54acfbfed336b84144256fb7ff8/datafusion/core/src/execution/memory_manager/mod.rs#L331-L335

@alamb
Copy link
Contributor

alamb commented Dec 19, 2022

I think this is now complete with #4351 and #4522

@alamb alamb closed this as completed Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants