Conversation
The extra RAII class existed only for callers who enable iostream failbit/badbit exceptions. Default streams do not throw; write/load failures already return Status via !good() / peek. Restoring the old mask could itself throw on eofbit. Keep the Status checks. Document that *NoExceptions I/O assumes the default goodbit mask.
clear() did not reset label_lookup_, num_deleted_, or deleted_elements, so a second loadIndex kept stale labels and could replace a live point after allow_replace_deleted reload. Validate the stream header and on-disk layout before mutating the live index; a truncated or corrupt file now returns Status and leaves the existing graph in place. Bruteforce load rebuilds the label map and reads labels via memcpy. addPoint rolls back the reserved slot if link-list allocation fails and we are still the last element.
Throwing loadIndex is now a thin wrapper. Failures return Status without mutating the live index (unopened/truncated streams). Successful loads still replace data_ only after a full read and rebuild the label map.
Stacked on the stream-simplify commit, which removed the helper. Keep the same Status checks as HierarchicalNSW *NoExceptions I/O.
Status copied a borrowed const char*, so a stack buffer or temporary string became a dangling message(). Copy into std::string. HNSWLIB_ENABLE_EXCEPTIONS=OFF no longer FORCE-writes CMAKE_CXX_FLAGS_* without /EHsc, which leaked into parent projects when examples were on. Exception flags stay per-target. The option does not export -fno-exceptions on the INTERFACE library.
After a failed loadIndexNoExceptions the object looked empty (cur_element_count == 0) but kept the incoming capacity with data_level0_memory_ == nullptr. addPoint then passed the capacity check and wrote through a null pointer. Also bound the link-list free loop to element_levels_.size().
Bruteforce already failed closed on that header. HNSW allocated file_max_elements slots and then read file_cur_count vectors, which overflows when the two counts disagree. Also reject a zero size_data_per_element before the layout walk.
element_levels_[cur_c] was already the intended level. On malloc failure that left a null linkLists_[cur_c] inside the published range, so saveIndex wrote from nullptr. Always drop the level to 0 first; fully unpublish the slot only when it is still last.
HNSW still builds mutex vectors, VisitedListPool, and label maps after clear(). Catch std::bad_alloc and reset rather than escaping the no-exceptions API. Bruteforce rebuilds the label map on the new buffer before swapping it in, so a map allocation failure leaves the live index untouched.
Owning the message via std::string fixes dangling stack pointers. Typical error strings exceed SSO, so -fno-exceptions builds can still abort while reporting an error. Callers should pass string literals.
The file stores size_per_element but load recomputed it from the Space and ignored the on-disk value, so a smaller dim still returned OK and read labels at the wrong packed offset.
CMAKE_CXX_FLAGS already has /EHsc. Per-target OFF only set /GR- and _HAS_EXCEPTIONS=0, so EH codegen stayed on. /EHs-c- overrides the default on the target and does not FORCE-write parent flags.
Keep the current invariant: do not mutate the live index until the on-disk layout has been checked.
michaelbautin
requested changes
Sep 15, 2026
| if (file_size_per_element != size_per_element) { | ||
| return Status("Cannot load index: size_per_element does not match space"); | ||
| } | ||
| char *new_data = (char *) malloc(file_maxelements * size_per_element); |
Collaborator
There was a problem hiding this comment.
It would be preferable to use RAII to manage this allocation.
| } | ||
|
|
||
| std::unordered_map<labeltype, size_t> new_dict; | ||
| #if defined(__EXCEPTIONS) || _HAS_EXCEPTIONS == 1 |
Collaborator
There was a problem hiding this comment.
Instead of repeating defined(__EXCEPTIONS) || _HAS_EXCEPTIONS == 1 in multiple places, we could define a HNSWLIB_... macro, e.g.
In hnswlib.h:
#if defined(__EXCEPTIONS) || _HAS_EXCEPTIONS == 1
#define HNSWLIB_EXCEPTIONS_ENABLED 1
#else
#undef HNSWLIB_EXCEPTIONS_ENABLED
#endifThen here
#ifdef HNSWLIB_EXCEPTIONS_ENABLED
try {
#endifetc.
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
2 participants
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.
StreamExceptionsOff/invokeWithoutStreamThrow.*NoExceptionsI/O already returnsStatusvia!good()/peek. Callers must leave the default iostream mask (goodbit)clear()a live HNSW index until the on-disk layout checks out.clear()resets lookup / deleted set /max_elements_. Reload no longer keeps stale labels ordeleted_elementsBruteforceSearch::loadIndexNoExceptions; throwingloadIndexis a wrapper. Rebuild the label map; memcpy labels; rejectsize_per_elementmismatch vsSpaceStatuscopies the message (no dangling stackconst char*)FORCE-strip/EHscfrom the parent cache.HNSWLIB_ENABLE_EXCEPTIONSis example/test-only; MSVC OFF uses/EHs-c-on the target