[XrdPfc] Backstop crash from negative dir count in purge check - #54
Open
h2zh wants to merge 1 commit into
Open
Conversation
A negative root directory count, converted to size_t, makes vector::reserve() throw std::length_error and abort the daemon from the purge thread (xrootd#2808). Clamp n_calc_dirs to at least 1; reserve() is only a capacity hint and fill_pshot_vec_children() walks the actual tree, so purging proceeds correctly. This is only a backstop. The count drifts negative because the accounting is asymmetric: directory removals are counted whenever the empty-dir sweep unlinks a dir and erases its DirState node, but directory creations are only counted for cache-miss opens (!m_existing_file). A file written to disk out-of-band (e.g. the Pelican cache self-test file) re-creates the DirState nodes without ever being counted. The real fix should be counting m_NDirectoriesCreated whenever a file open has to create DirState nodes in memory, regardless of m_existing_file. (m_NFilesCreated must stay gated on !m_existing_file to avoid over-counting cache hits.) Having said that, upstream XRootD developers may have more to consider.
Collaborator
|
I built xrootd-5.9.6-1.2 with this patch; I haven't put it into osg-testing yet because we're having odd VMU test failures (that aren't related to this patch, but I'd like to get some clean tests before putting the new build into osg-testing). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A negative root directory count, converted to size_t, makes
vector::reserve() throw std::length_error and abort the daemon from
the purge thread (xrootd#2808). Clamp n_calc_dirs to at least
1; reserve() is only a capacity hint and fill_pshot_vec_children()
walks the actual tree, so purging proceeds correctly.
This is only a backstop. The count drifts negative because the
accounting is asymmetric: directory removals are counted whenever the
empty-dir sweep unlinks a dir and erases its DirState node, but
directory creations are only counted for cache-miss opens
(!m_existing_file). A file written to disk out-of-band (e.g. the
Pelican cache self-test file) re-creates the DirState nodes without
ever being counted. The real fix should be counting m_NDirectoriesCreated
whenever a file open has to create DirState nodes in memory,
regardless of m_existing_file. (m_NFilesCreated must stay gated on
!m_existing_file to avoid over-counting cache hits.) Having said that,
upstream XRootD developers may have more to consider.