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

modules/content: drop incorrect assertion #5781

Merged
merged 1 commit into from Mar 9, 2024

Conversation

garlick
Copy link
Member

@garlick garlick commented Mar 8, 2024

Problem: sometimes brokers hit an assertion when the content module is unloaded during shutdown.

The assertion is that there are no pending load/store operations waiting on a cache entry when it is destroyed. This is certainly possible, depending on the behavior of users of the load/store RPCs. In the case of unload in rc3, dependent modules would have already been unloaded, so it seems safe to simply destroy any pending RPCs without responding to them.

Destroy them, if present.

Fixes #5706

Problem: sometimes brokers hit an assertion when the
content module is unloaded during shutdown.

The assertion is that there are no pending load/store operations
waiting on a cache entry when it is destroyed.  This is certainly
possible, depending on the behavior of users of the load/store
RPCs.  In the case of unload in rc3, dependent modules would have
already been unloaded, so it seems safe to simply destroy any
pending RPCs without responding to them.

Destroy them, if present.

Fixes flux-framework#5706
Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

Copy link

codecov bot commented Mar 9, 2024

Codecov Report

Merging #5781 (b53f8ee) into master (0a8e91a) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5781   +/-   ##
=======================================
  Coverage   83.33%   83.33%           
=======================================
  Files         509      509           
  Lines       82486    82486           
=======================================
  Hits        68737    68737           
  Misses      13749    13749           
Files Coverage Δ
src/modules/content/cache.c 83.33% <100.00%> (ø)

... and 16 files with indirect coverage changes

@mergify mergify bot merged commit 00a7970 into flux-framework:master Mar 9, 2024
35 checks passed
@garlick garlick deleted the issue#5706 branch March 9, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

broker crash in content_cache_destroy
2 participants