ReadAnchor throws exception on second run #29931

issue asctime openend this issue on April 21, 2024
  1. asctime commented at 7:27 pm on April 21, 2024: none

    Is there an existing issue for this?

    • I have searched the existing issues

    Current behaviour

     0std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
     1{
     2    std::vector<CAddress> anchors;
     3    try {
     4        DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
     5        LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
     6    } catch (const std::exception&) {
     7        anchors.clear();
     8    }
     9
    10    fs::remove(anchors_db_path);
    11    return anchors;
    12}
    

    On second run I would get: EXCEPTION: NSt10filesystem7__cxx1116filesystem_errorE
    filesystem error: cannot remove: No such file or directory [… \AppData\Roaming\Bitcoin\anchors.dat]
    ..\bitcoin-qt.exe in Runaway exception

    Attempting blind delete on already-deleted file..

    Expected behaviour

    No crash on second run, this works for me:

     0std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path)
     1{
     2    std::vector<CAddress> anchors;
     3    try {
     4        DeserializeFileDB(anchors_db_path, CAddress::V2_DISK(anchors));
     5        LogPrintf("Loaded %i addresses from %s\n", anchors.size(), fs::quoted(fs::PathToString(anchors_db_path.filename())));
     6    } catch (const std::exception&) {
     7        anchors.clear();
     8    }
     9
    10    if (fs::exists(anchors_db_path)) {
    11        try {
    12            fs::remove(anchors_db_path);
    13        } catch (const std::filesystem::filesystem_error& e) {
    14            LogPrintf("Error. %s could not be deleted: %s\n", fs::quoted(fs::PathToString(anchors_db_path.filename())), e.what());
    15        }
    16    }
    17    return anchors;
    18}
    

    I emphasize that I am not very familiar with the code base, and there could be reasons for the way this is done. Just trying to better understand it really..

    Steps to reproduce

    Compile from source MinGW64, run twice..

    Relevant log output

    No response

    How did you obtain Bitcoin Core

    Compiled from source

    What version of Bitcoin Core are you using?

    26.1

    Operating system and version

    Windows, MinGW64 (using portable_endian.h)

    Machine specifications

    No response

  2. maflcko commented at 8:07 am on April 22, 2024: member

    Compile from source MinGW64, run twice..

    What are the exact steps to reproduce, including the exact operating system version?

    Does it happen with the compiled release version as well?

    Ref: https://en.cppreference.com/w/cpp/filesystem/remove

  3. laanwj added the label P2P on Apr 22, 2024
  4. asctime commented at 11:29 pm on April 22, 2024: none
    Windows 10. I run the official binary on Linux and that’s fine. Different compilers seem to manage the exception slightly differently? Regardless I’m not sure that a blind delete is the right way to do this, is all. Fixed on my side, I just wanted to point it out.
  5. maflcko commented at 4:58 am on April 23, 2024: member

    Different compilers seem to manage the exception slightly differently?

    That should not be the case. The point of standard C++ library std::filesystem::remove is to provide the same interface behavior, regardless of compiler or operating system.

    The specification says that false should be returned when the file does not exist, not an exception be thrown.

    However, without exact steps to reproduce, using your compiler version, there is little that can be done here.

    Regardless I’m not sure that a blind delete is the right way to do this, is all.

    Right, it could make sense to catch the exception for other reasons, or not allow it to be thrown, but that is a different question.

  6. maflcko commented at 4:59 am on April 23, 2024: member
    According to #29930 (comment) the reason could be that you are using an experimental C++17 standard library, which has bugs?
  7. asctime commented at 7:20 am on April 23, 2024: none

    That should not be the case. The point of standard C++ library std::filesystem::remove is to provide the same interface behavior, regardless of compiler or operating system.

    The specification says that false should be returned when the file does not exist, not an exception be thrown.

    Ah good point ><. I’ll try it once with my clang install, it’s a fair bit newer than my gcc which is due to be updated anyway. Ok to close from my side. Thanks.

  8. maflcko commented at 7:46 am on April 23, 2024: member
    Closing for now, but please leave a comment if there are more details to debug this issue.
  9. maflcko closed this on Apr 23, 2024


asctime maflcko

Labels
P2P


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me