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

HttpBinaryCacheStore::getFile(): Fix uncaught exception #11600

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Sep 26, 2024

Motivation

This method is marked as noexcept, but enqueueFileTransfer() can throw Interrupted if the user has hit Ctrl-C or if the ThreadPool that the thread is a part of is shutting down.

Should fix DeterminateSystems/magic-nix-cache#101, #11511.

Here is the thread that hits noexcept:

#0  0x0000ffaa3c047240 __pthread_kill_implementation
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x87240
    /build/glibc-2.39/nptl/pthread_kill.c:44:76
#1  0x0000ffaa3c0001bc raise
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x401bb
    ../sysdeps/posix/raise.c:26:13
#2  0x0000ffaa3bfebe3c abort
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x2be3b
    /build/glibc-2.39/stdlib/abort.c:79:7
#3  0x0000ffaa3c312778 __gnu_cxx::__verbose_terminate_handler()
#4  0x0000ffaa3c3101fc __cxxabiv1::__terminate(void (*)())
#5  0x0000ffaa3c30f114 __cxa_call_terminate
#6  0x0000ffaa3c30f8fc __gxx_personality_v0
#7  0x0000ffaa3c180efc _Unwind_RaiseException_Phase2
#8  0x0000ffaa3c181484 _Unwind_Resume
#9  0x0000ffaa3c99bda8 nix::curlFileTransfer::enqueueFileTransfer(nix::FileTransferRequest const&, nix::Callback<nix::FileTransferResult>)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1ebda7
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1071:21
#10 0x0000ffaa3c9d48f4 nix::HttpBinaryCacheStore::getFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, nix::Callback<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x2248f3
    src/libstore/http-binary-cache-store.cc:181:47
#11 0x0000ffaa3c8c7dd0 nix::BinaryCacheStore::queryPathInfoUncached(nix::StorePath const&, nix::Callback<std::shared_ptr<nix::ValidPathInfo const> >)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x117dcf
    src/libstore/binary-cache-store.cc:425:12
#12 0x0000ffaa3cac0d8c nix::Store::queryPathInfo(nix::StorePath const&, nix::Callback<nix::ref<nix::ValidPathInfo const> >)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x310d8b
    src/libstore/store-api.cc:693:26
#13 0x0000ffaa3cac1118 nix::Store::queryPathInfo(nix::StorePath const&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x311117
    src/libstore/store-api.cc:618:18
#14 0x0000ffaa3cac82c0 nix::Store::querySubstitutablePathInfos(std::map<nix::StorePath, std::optional<nix::ContentAddress>, std::less<nix::StorePath>, std::allocator<std::pair<nix::StorePath const, std::optional<nix::ContentAddress> > > > const&, std::map<nix::StorePath, nix::SubstitutablePathInfo, std::less<nix::StorePath>, std::allocator<std::pair<nix::StorePath const, nix::SubstitutablePathInfo> > >&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x3182bf
    src/libstore/store-api.cc:543:55
#15 0x0000ffaa3ca27f70 std::_Function_handler<void (nix::DerivedPath), nix::Store::queryMissing(std::vector<nix::DerivedPath, std::allocator<nix::DerivedPath> > const&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, unsigned long&, unsigned long&)::{lambda(nix::DerivedPath const&)#1}>::_M_invoke(std::_Any_data const&, nix::DerivedPath&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x277f6f
    src/libstore/misc.cc:269:40
#16 0x0000ffaa3ca2bd4c std::_Function_handler<void (), std::_Bind<std::function<void (nix::DerivedPath)> (nix::DerivedPathOpaque)> >::_M_invoke(std::_Any_data const&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x27bd4b
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#17 0x0000ffaa3c70ace0 nix::ThreadPool::doWork(bool)
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x14acdf
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#18 0x0000ffaa3c33d30c execute_native_thread_routine
#19 0x0000ffaa3c04566c start_thread
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x8566b
    /build/glibc-2.39/nptl/pthread_create.c:447:8
#20 0x0000ffaa3c0a8c8c thread_start
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0xe8c8b
    ../sysdeps/unix/sysv/linux/aarch64/clone3.S:76

And it got an Interrupt exception because the thread pool is shutting down:

#0  0x0000ffaa3c041e4c __GI___futex_abstimed_wait_cancelable64
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x81e4c
    /build/glibc-2.39/nptl/futex-internal.c:57:12
#1  0x0000ffaa3c046f50 __pthread_clockjoin_ex
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x86f4f
    /build/glibc-2.39/nptl/pthread_join_common.c:102:14
#2  0x0000ffaa3c33d3b0 std::thread::join()
#3  0x0000ffaa3c70ce00 nix::ThreadPool::shutdown()
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x14cdff
    src/libutil/thread-pool.cc:39:17
#4  0x0000ffaa3c70d2dc nix::ThreadPool::process()
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x14d2db
    src/libutil/thread-pool.cc:75:17
#5  0x0000ffaa3ca25b60 nix::Store::queryMissing(std::vector<nix::DerivedPath, std::allocator<nix::DerivedPath> > const&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, std::set<nix::StorePath, std::less<nix::StorePath>, std::allocator<nix::StorePath> >&, unsigned long&, unsigned long&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x275b5f
    src/libstore/misc.cc:296:17
#6  0x0000ffaa3c95a6c0 nix::daemon::performOp(nix::daemon::TunnelLogger*, nix::ref<nix::Store>, nix::TrustedFlag, nix::daemon::RecursiveFlag, nix::WorkerProto::BasicServerConnection&, nix::WorkerProto::Op)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1aa6bf
    src/libstore/daemon.cc:949:28
#7  0x0000ffaa3c95dd44 nix::daemon::processConnection(nix::ref<nix::Store>, nix::FdSource&&, nix::FdSink&&, nix::TrustedFlag, nix::daemon::RecursiveFlag)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1add43
    src/libstore/daemon.cc:1089:26
#8  0x0000000000605980 std::_Function_handler<void (), daemonLoop(std::optional<nix::TrustedFlag>)::{lambda()#1}>::_M_invoke(std::_Any_data const&)
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x20597f
    src/nix/unix/daemon.cc:373:34
#9  0x0000ffaa3c719f0c std::_Function_handler<void (), nix::startProcess(std::function<void ()>, nix::ProcessOptions const&)::{lambda()#1}>::_M_invoke(std::_Any_data const&)
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x159f0b
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#10 0x0000ffaa3c718c54 nix::doFork(bool, std::function<void ()>&) [clone .constprop.0]
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x158c53
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#11 0x0000ffaa3c71a154 nix::startProcess(std::function<void ()>, nix::ProcessOptions const&)
    [d273e69e0bbf1dbc9d7f2e07976660f84a06e3cf]@0xffaa3c5c0000+0x15a153
    src/libutil/unix/processes.cc:240:21
#12 0x0000000000606250 daemonLoop(std::optional<nix::TrustedFlag>)
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x20624f
    src/nix/unix/daemon.cc:356:25
#13 0x00000000006079c8 main_nix_daemon(int, char**)
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x2079c7
    src/nix/unix/daemon.cc:515:18
#14 0x000000000058effc nix::mainWrapped(int, char**)
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x18effb
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#15 0x0000ffaa3ce894d8 nix::handleExceptions(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void ()>)
    [250099236b28230e5bb4c44be3cc53a21ab6d9cf]@0xffaa3ce50000+0x394d7
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#16 0x0000000000453438 main
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x53437
    src/nix/main.cc:551:33
#17 0x0000ffaa3bfec500 __libc_start_call_main
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x2c4ff
    ../sysdeps/nptl/libc_start_call_main.h:58:16
#18 0x0000ffaa3bfec5d8 __libc_start_main@@GLIBC_2.34
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x2c5d7
    ../csu/libc-start.c:360:3
#19 0x00000000004598b0 _start
    [76f19933e61ad8826ee408981a3ca0772055bb7a]@0x400000+0x598af

And the ThreadPool is shutting down because some binary cache lookups failed:

#0  0x0000ffaa3c084c04 __libc_pwrite
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0xc4c04
    ../sysdeps/unix/sysv/linux/pwrite64.c:25:10
#1  0x0000ffaa3b7a0ac4 seekAndWriteFd
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0x20ac3
    /build/sqlite-autoconf-3450300/sqlite3.c:41489:17
#2  0x0000ffaa3b7a0b58 unixWrite
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0x20b57
    /build/sqlite-autoconf-3450300/sqlite3.c:41520:10
#3  0x0000ffaa3b80acc4 writeJournalHdr.constprop.0
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0x8acc3
    /build/sqlite-autoconf-3450300/sqlite3.c:25869:10
#4  0x0000ffaa3b80b144 pager_write
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0x8b143
    /build/sqlite-autoconf-3450300/sqlite3.c:62629:12
#5  0x0000ffaa3b81abc8 sqlite3BtreeInsert
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0x9abc7
    /build/sqlite-autoconf-3450300/sqlite3.c:77504:14
#6  0x0000ffaa3b849d48 sqlite3VdbeExec
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0xc9d47
    /build/sqlite-autoconf-3450300/sqlite3.c:99117:8
#7  0x0000ffaa3b84e92c sqlite3_step
    [103e7f2afdb88a6761ef02b754a72fa84a7fa1ee]@0xffaa3b780000+0xce92b
    /build/sqlite-autoconf-3450300/sqlite3.c:90771:10
#8  0x0000ffaa3caa824c nix::SQLiteStmt::Use::exec()
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x2f824b
    src/libstore/sqlite.cc:190:17
#9  0x0000ffaa3ca3d318 nix::NarInfoDiskCacheImpl::upsertNarInfo(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::shared_ptr<nix::ValidPathInfo const>)::{lambda()#1}::operator()() const
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x28d317
    src/libstore/nar-info-disk-cache.cc:352:35
#10 0x0000ffaa3ca3d704 nix::NarInfoDiskCacheImpl::upsertNarInfo(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::shared_ptr<nix::ValidPathInfo const>)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x28d703
    src/libstore/sqlite.hh:176:23
#11 0x0000ffaa3cac1824 nix::Store::queryPathInfo(nix::StorePath const&, nix::Callback<nix::ref<nix::ValidPathInfo const> >)::{lambda(std::future<std::shared_ptr<nix::ValidPathInfo const> >)#1}::operator()(std::future<std::shared_ptr<nix::ValidPathInfo const> >) const
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x311823
    src/libstore/store-api.cc:700:45
#12 0x0000ffaa3cac1c2c std::_Function_handler<void (std::future<std::shared_ptr<nix::ValidPathInfo const> >), nix::Store::queryPathInfo(nix::StorePath const&, nix::Callback<nix::ref<nix::ValidPathInfo const> >)::{lambda(std::future<std::shared_ptr<nix::ValidPathInfo const> >)#1}>::_M_invoke(std::_Any_data const&, std::future<std::shared_ptr<nix::ValidPathInfo const> >&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x311c2b
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:61:36
#13 0x0000ffaa3c8d6cd8 nix::Callback<std::shared_ptr<nix::ValidPathInfo const> >::operator()(std::shared_ptr<nix::ValidPathInfo const>&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x126cd7
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#14 0x0000ffaa3c8c4a4c std::_Function_handler<void (std::future<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >), nix::BinaryCacheStore::queryPathInfoUncached(nix::StorePath const&, nix::Callback<std::shared_ptr<nix::ValidPathInfo const> >)::{lambda(std::future<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)#1}>::_M_invoke(std::_Any_data const&, std::future<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x114a4b
    src/libstore/binary-cache-store.cc:430:49
#15 0x0000ffaa3c9d3a18 nix::Callback<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::operator()(std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x223a17
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#16 0x0000ffaa3c9d56f0 std::_Function_handler<void (std::future<nix::FileTransferResult>), nix::HttpBinaryCacheStore::getFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, nix::Callback<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::{lambda(std::future<nix::FileTransferResult>)#1}>::_M_invoke(std::_Any_data const&, std::future<nix::FileTransferResult>&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x2256ef
    src/libstore/http-binary-cache-store.cc:187:46
#17 0x0000ffaa3c998c90 nix::Callback<nix::FileTransferResult>::rethrow(std::__exception_ptr::exception_ptr const&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1e8c8f
    /nix/store/m1x0zcvlj5jvgzbxzl8n53qjr5kbfb0y-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
#18 0x0000ffaa3c999028 void nix::curlFileTransfer::TransferItem::fail<nix::FileTransferError>(nix::FileTransferError&&)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1e9027
    src/libstore/filetransfer.cc:148:29
#19 0x0000ffaa3c99d88c nix::curlFileTransfer::TransferItem::finish(CURLcode)
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1ed88b
    src/libstore/filetransfer.cc:509:25
#20 0x0000ffaa3c99ef40 nix::curlFileTransfer::workerThreadMain()
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1eef3f
    src/libstore/filetransfer.cc:621:38
#21 0x0000ffaa3c99f9e8 nix::curlFileTransfer::workerThreadEntry()
    [40d4e5dad10dcb9cde5bd45875782286bf741556]@0xffaa3c7b0000+0x1ef9e7
    src/libstore/filetransfer.cc:693:29
#22 0x0000ffaa3c33d30c execute_native_thread_routine
#23 0x0000ffaa3c04566c start_thread
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0x8566b
    /build/glibc-2.39/nptl/pthread_create.c:447:8
#24 0x0000ffaa3c0a8c8c thread_start
    [9408142a336561d9745a7ccb74d94f41dc7c09fb]@0xffaa3bfc0000+0xe8c8b
    ../sysdeps/unix/sysv/linux/aarch64/clone3.S:76

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This method is marked as `noexcept`, but `enqueueFileTransfer()` can
throw `Interrupted` if the user has hit Ctrl-C or if the `ThreadPool`
that the thread is a part of is shutting down.
@edolstra edolstra added bug backport 2.24-maintenance Automatically creates a PR against the branch labels Sep 26, 2024
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Sep 26, 2024
Copy link
Member

@layus layus left a comment

Choose a reason for hiding this comment

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

This makes sense. It's a bit sad that compilers cannot enforce noexcept at compile time. Clang tidy seems to have an option for that. Maybe worth exploring (see https://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-escape.html).

I wonder if more of these may be lurking in the dark 🤔, but see no reason not to merge this one in particular ASAP.

@edolstra edolstra merged commit 08deebd into NixOS:master Sep 27, 2024
12 checks passed
@edolstra edolstra deleted the fix-uncaught-exception branch September 27, 2024 10:37
@edolstra
Copy link
Member Author

In this particular case, maybe we should just remove the noexcept. It's intended to ensure that the caller only has to deal with exceptions via the Callback, but that might not be a big deal.

@layus
Copy link
Member

layus commented Sep 27, 2024

True. An incorrect specification is worse than no specification at all.

edolstra added a commit that referenced this pull request Sep 27, 2024
…1600

HttpBinaryCacheStore::getFile(): Fix uncaught exception (backport #11600)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.24-maintenance Automatically creates a PR against the branch bug store Issues and pull requests concerning the Nix store
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Could GitHub Actions Cache throttling lead to nix crashes ?
2 participants