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.

Sign up for GitHub

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

Jump to bottom

LibCore+RS+LibCrypto: Make RS (crash) faster #24313

Merged
merged 10 commits into from
May 20, 2024
Merged

LibCore+RS+LibCrypto: Make RS (crash) faster #24313

merged 10 commits into from
May 20, 2024

Conversation

alimpfard
Copy link
Member

Continuation of #24265.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 13, 2024
@alimpfard
Copy link
Member Author

alimpfard commented May 13, 2024 •
edited

Note that there is some memory corruption I haven't tracked down yet, so pls no marge until I fix. fixed.

@alimpfard alimpfard force-pushed the rs-wao branch 5 times, most recently from 21804db to e3fbd38 Compare May 14, 2024 15:26
@BuggieBot
Copy link
Member

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

Zaggy1024
Zaggy1024 reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@Zaggy1024 Zaggy1024 left a comment

Choose a reason for hiding this comment

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

The video changes look really nice!

Userland/Libraries/LibVideo/VP9/Parser.cpp Outdated Show resolved Hide resolved
@awesomekling
Copy link
Member

Hit this within seconds when trying to load twitter.com:

VERIFICATION FAILED: it != thread_data.notifier_by_ptr.end() at /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp:675
/home/kling/src/serenity/Build/lagom/lib/liblagom-ak.so.0(ak_verification_failed+0xef) [0x792814bfc2ff]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::EventLoopManagerUnix::unregister_notifier(Core::Notifier&) 0x17e) [0x792814ce8afe]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::Notifier::set_enabled(bool) 0x3e) [0x792814cf315e]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0(+0x84cec) [0x792814cfecec]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x4d2b5) [0x60544d58e2b5]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x331ef) [0x60544d5741ef]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x39bdd) [0x60544d57abdd]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0(+0x7305f) [0x792814ced05f]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::ThreadEventQueue::process() 0x1da) [0x792814d02c3a]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::EventLoopImplementationUnix::exec() 0x2c) [0x792814ce69dc]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::EventLoop::exec() 0x23) [0x792814ce4fe3]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x3f448) [0x60544d580448]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x3f21b) [0x60544d58021b]
/home/kling/src/serenity/Build/lagom/lib/liblagom-threading.so.0(+0x6325) [0x792814d21325]
/home/kling/src/serenity/Build/lagom/lib/liblagom-threading.so.0(+0x6128) [0x792814d21128]
/lib/x86_64-linux-gnu/libc.so.6(+0x9ca94) [0x79281449ca94]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c) [0x792814529c3c]

@awesomekling
Copy link
Member

Built with sanitizers, hit some more errors:

==3333569==ERROR: AddressSanitizer: heap-use-after-free on address 0x503000022fcc at pc 0x7a5cba73882a bp 0x7a5c78bff140 sp 0x7a5c78bff138
READ of size 1 at 0x503000022fcc thread T39
    #0 0x7a5cba738829 in AK::Detail::StringData::hash() const /home/kling/src/serenity/Meta/Lagom/../../AK/StringData.h:82:14
    #1 0x7a5cbad3a65c in hash /home/kling/src/serenity/Meta/Lagom/../../AK/HashMap.h:29:59
    #2 0x7a5cbad3a65c in AK::HashSetResult AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false>::write_value<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&>(AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&, AK::HashSetExistingEntryBehavior) /home/kling/src/serenity/Meta/Lagom/../../AK/HashTable.h:647:29
    #3 0x7a5cbad3a3bf in AK::ErrorOr<AK::HashSetResult, AK::Error> AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false>::try_set<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&>(AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&, AK::HashSetExistingEntryBehavior) /home/kling/src/serenity/Meta/Lagom/../../AK/HashTable.h:365:16
    #4 0x7a5cbad3a12d in AK::HashSetResult AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false>::set<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&>(AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry const&, AK::HashSetExistingEntryBehavior) /home/kling/src/serenity/Meta/Lagom/../../AK/HashTable.h:370:16
    #5 0x7a5cbad39e1f in AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false>::HashTable(AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false> const&) /home/kling/src/serenity/Meta/Lagom/../../AK/HashTable.h:177:13
    #6 0x7a5cbad3755c in HashMap /home/kling/src/serenity/Meta/Lagom/../../AK/HashMap.h:46:5
    #7 0x7a5cbad3755c in RelativeDistinguishedName /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibTLS/Certificate.h:193:7
    #8 0x7a5cbad3755c in TLS::Certificate::Certificate(TLS::Certificate const&) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibTLS/Certificate.h:262:7
    #9 0x7a5cbadf1b3c in AK::TypedTransfer<TLS::Certificate>::copy(TLS::Certificate*, TLS::Certificate const*, unsigned long) /home/kling/src/serenity/Meta/Lagom/../../AK/TypedTransfer.h
    #10 0x7a5cbadc27a7 in Vector /home/kling/src/serenity/Meta/Lagom/../../AK/Vector.h:88:9
    #11 0x7a5cbadc27a7 in TLS::TLSv12::TLSv12(AK::Variant<AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>, Core::Socket*>, TLS::Options) /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:517:27
    #12 0x7a5cbadaa562 in AK::NonnullOwnPtr<TLS::TLSv12> AK::make<TLS::TLSv12, AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>, TLS::Options>(AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>&&, TLS::Options&&) /home/kling/src/serenity/Meta/Lagom/../../AK/NonnullOwnPtr.h:150:59
    #13 0x7a5cbada155d in TLS::TLSv12::connect(AK::ByteString const&, unsigned short, TLS::Options) /home/kling/src/serenity/Userland/Libraries/LibTLS/Socket.cpp:58:23
    #14 0x5c9d3ae98c7c in AK::ErrorOr<AK::NonnullOwnPtr<TLS::TLSv12>, AK::Error> RequestServer::ConnectionCache::Proxy::tunnel<TLS::TLSv12, TLS::TLSv12>(URL::URL const&) /home/kling/src/serenity/Meta/Lagom/../../Userland/Services/RequestServer/ConnectionCache.h:41:20
    #15 0x5c9d3af4e589 in void RequestServer::ConnectionCache::ensure_connection<Threading::RWLockProtected<AK::HashMap<RequestServer::ConnectionCache::ConnectionKey, AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>, AK::Traits<RequestServer::ConnectionCache::ConnectionKey>, AK::Traits<AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>>, false>>, AK::NonnullRefPtr<HTTP::HttpsJob>>(Threading::RWLockProtected<AK::HashMap<RequestServer::ConnectionCache::ConnectionKey, AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>, AK::Traits<RequestServer::ConnectionCache::ConnectionKey>, AK::Traits<AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>>, false>>&, URL::URL const&, AK::NonnullRefPtr<HTTP::HttpsJob>, Core::ProxyData) /home/kling/src/serenity/Meta/Lagom/../../Userland/Services/RequestServer/ConnectionCache.h:271:40
    #16 0x5c9d3af4ddf5 in AK::OwnPtr<RequestServer::Request, AK::DefaultDelete<RequestServer::Request>> RequestServer::Detail::start_request<AK::Badge<RequestServer::HttpsProtocol>, AK::ErrorOr<RequestServer::Protocol::Pipe, AK::Error>>(AK::Badge<RequestServer::HttpsProtocol>&&, int, RequestServer::ConnectionFromClient&, AK::ByteString const&, URL::URL const&, AK::HashMap<AK::ByteString, AK::ByteString, AK::Traits<AK::ByteString>, AK::Traits<AK::ByteString>, false> const&, AK::Span<unsigned char const>, AK::ErrorOr<RequestServer::Protocol::Pipe, AK::Error>&&, Core::ProxyData)::'lambda'()::operator()() const /home/kling/src/serenity/Meta/Lagom/../../Userland/Services/RequestServer/HttpCommon.h:107:13
    #17 0x7a5cba956ecf in AK::Function<void ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #18 0x7a5cba9c0f7d in Core::ThreadEventQueue::process() /home/kling/src/serenity/Userland/Libraries/LibCore/ThreadEventQueue.cpp:110:17
    #19 0x7a5cba93b771 in Core::EventLoopImplementationUnix::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp:299:9
    #20 0x7a5cba934ea1 in Core::EventLoop::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoop.cpp:88:20
    #21 0x5c9d3aeccaf1 in RequestServer::ConnectionFromClient::Looper<Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>>::next(Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>&, bool) /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:94:16
    #22 0x5c9d3aecc591 in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::initialize_workers(unsigned long)::'lambda'()::operator()() const /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:116:49
    #23 0x7a5cbb6dc04f in AK::Function<long ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #24 0x7a5cbb6db29b in operator() /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:83:30
    #25 0x7a5cbb6db29b in Threading::Thread::start()::$_0::__invoke(void*) /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:80:9
    #26 0x5c9d3ae0f6ae in asan_thread_start(void*) crtstuff.c
    #27 0x7a5cba09ca93 in start_thread nptl/pthread_create.c:447:8
    #28 0x7a5cba129c3b in clone3 misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

0x503000022fcc is located 12 bytes inside of 24-byte region [0x503000022fc0,0x503000022fd8)
freed by thread T35 here:
    #0 0x5c9d3ae1178a in free (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x1c978a) (BuildId: 6d8cf7d79f6603ca)
    #1 0x7a5cba7380af in kfree_sized /home/kling/src/serenity/Meta/Lagom/../../AK/kmalloc.h:25:5
    #2 0x7a5cba7380af in operator delete /home/kling/src/serenity/Meta/Lagom/../../AK/StringData.h:47:9
    #3 0x7a5cba7380af in AK::RefCounted<AK::Detail::StringData>::unref() const /home/kling/src/serenity/Meta/Lagom/../../AK/RefCounted.h:65:13
    #4 0x7a5cbad136bd in ~StringBase /home/kling/src/serenity/Meta/Lagom/../../AK/StringBase.h:55:13
    #5 0x7a5cbad136bd in ~Entry /home/kling/src/serenity/Meta/Lagom/../../AK/HashMap.h:23:12
    #6 0x7a5cbad136bd in AK::HashTable<AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::Entry, AK::HashMap<AK::String, AK::String, AK::Traits<AK::String>, AK::Traits<AK::String>, false>::EntryTraits, false>::~HashTable() /home/kling/src/serenity/Meta/Lagom/../../AK/HashTable.h:166:43
    #7 0x7a5cbad02084 in ~HashMap /home/kling/src/serenity/Meta/Lagom/../../AK/HashMap.h:21:7
    #8 0x7a5cbad02084 in ~RelativeDistinguishedName /home/kling/src/serenity/Userland/Libraries/LibTLS/Certificate.h:193:7
    #9 0x7a5cbad02084 in TLS::Certificate::~Certificate() /home/kling/src/serenity/Userland/Libraries/LibTLS/Certificate.h:262:7
    #10 0x7a5cbadae4a3 in clear_with_capacity /home/kling/src/serenity/Meta/Lagom/../../AK/Vector.h:383:24
    #11 0x7a5cbadae4a3 in AK::Vector<TLS::Certificate, 0ul>::clear() /home/kling/src/serenity/Meta/Lagom/../../AK/Vector.h:372:9
    #12 0x7a5cbadc27ed in ~Vector /home/kling/src/serenity/Meta/Lagom/../../AK/Vector.h:110:9
    #13 0x7a5cbadc27ed in TLS::TLSv12::TLSv12(AK::Variant<AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>, Core::Socket*>, TLS::Options) /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:517:5
    #14 0x7a5cbadaa562 in AK::NonnullOwnPtr<TLS::TLSv12> AK::make<TLS::TLSv12, AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>, TLS::Options>(AK::OwnPtr<Core::Socket, AK::DefaultDelete<Core::Socket>>&&, TLS::Options&&) /home/kling/src/serenity/Meta/Lagom/../../AK/NonnullOwnPtr.h:150:59
    #15 0x7a5cbada155d in TLS::TLSv12::connect(AK::ByteString const&, unsigned short, TLS::Options) /home/kling/src/serenity/Userland/Libraries/LibTLS/Socket.cpp:58:23
    #16 0x5c9d3ae98c7c in AK::ErrorOr<AK::NonnullOwnPtr<TLS::TLSv12>, AK::Error> RequestServer::ConnectionCache::Proxy::tunnel<TLS::TLSv12, TLS::TLSv12>(URL::URL const&) /home/kling/src/serenity/Meta/Lagom/../../Userland/Services/RequestServer/ConnectionCache.h:41:20
    #17 0x5c9d3ae96569 in void RequestServer::ConnectionCache::ensure_connection<Threading::RWLockProtected<AK::HashMap<RequestServer::ConnectionCache::ConnectionKey, AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>, AK::Traits<RequestServer::ConnectionCache::ConnectionKey>, AK::Traits<AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>>, false>>, AK::NonnullRefPtr<RequestServer::Job>>(Threading::RWLockProtected<AK::HashMap<RequestServer::ConnectionCache::ConnectionKey, AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>, AK::Traits<RequestServer::ConnectionCache::ConnectionKey>, AK::Traits<AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12>>, 0ul>>>, false>>&, URL::URL const&, AK::NonnullRefPtr<RequestServer::Job>, Core::ProxyData) /home/kling/src/serenity/Meta/Lagom/../../Userland/Services/RequestServer/ConnectionCache.h:271:40
    #18 0x5c9d3ae57052 in operator()<Threading::RWLockProtected<AK::HashMap<RequestServer::ConnectionCache::ConnectionKey, AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12> >, 0UL> >, AK::Traits<RequestServer::ConnectionCache::ConnectionKey>, AK::Traits<AK::NonnullOwnPtr<AK::Vector<AK::NonnullOwnPtr<RequestServer::ConnectionCache::Connection<TLS::TLSv12, TLS::TLSv12> >, 0UL> > >, false> > > /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:140:17
    #19 0x5c9d3ae57052 in operator() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:146:17
    #20 0x5c9d3ae57052 in visit<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>::Visitor<(lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:104:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:122:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:150:9)>, (unsigned char)'\x01'> /home/kling/src/serenity/Meta/Lagom/../../AK/Variant.h:114:20
    #21 0x5c9d3ae57052 in visit<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>::Visitor<(lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:104:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:122:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:150:9)>, (unsigned char)'\x00'> /home/kling/src/serenity/Meta/Lagom/../../AK/Variant.h:118:20
    #22 0x5c9d3ae57052 in visit<(lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:104:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:122:9), (lambda at /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:150:9)> /home/kling/src/serenity/Meta/Lagom/../../AK/Variant.h:423:16
    #23 0x5c9d3ae57052 in RequestServer::ConnectionFromClient::worker_do_work(AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>) /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:103:10
    #24 0x5c9d3ae5f99c in operator() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:29:41
    #25 0x5c9d3ae5f99c in AK::Function<void (AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>)>::CallableWrapper<RequestServer::ConnectionFromClient::ConnectionFromClient(AK::NonnullOwnPtr<Core::LocalSocket>)::$_0>::call(AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>) /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:192:20
    #26 0x5c9d3aece852 in AK::Function<void (AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>)>::operator()(AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>) const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #27 0x5c9d3aecdf58 in Threading::ThreadPoolLooper<Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>>::next(Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>&, bool) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:44:9
    #28 0x5c9d3aeccea3 in operator() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:81:48
    #29 0x5c9d3aeccea3 in AK::Function<void ()>::CallableWrapper<RequestServer::ConnectionFromClient::Looper<Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>>::next(Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>&, bool)::'lambda'()>::call() /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:192:20
    #30 0x7a5cba956ecf in AK::Function<void ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #31 0x7a5cba953a72 in Core::EventReceiver::dispatch_event(Core::Event&, Core::EventReceiver*) /home/kling/src/serenity/Userland/Libraries/LibCore/EventReceiver.cpp:162:17
    #32 0x7a5cba9c10d0 in Core::ThreadEventQueue::process() /home/kling/src/serenity/Userland/Libraries/LibCore/ThreadEventQueue.cpp:115:23
    #33 0x7a5cba93b771 in Core::EventLoopImplementationUnix::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp:299:9
    #34 0x7a5cba934ea1 in Core::EventLoop::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoop.cpp:88:20
    #35 0x5c9d3aeccaf1 in RequestServer::ConnectionFromClient::Looper<Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>>::next(Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>&, bool) /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:94:16
    #36 0x5c9d3aecc591 in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::initialize_workers(unsigned long)::'lambda'()::operator()() const /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:116:49
    #37 0x7a5cbb6dc04f in AK::Function<long ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #38 0x7a5cbb6db29b in operator() /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:83:30
    #39 0x7a5cbb6db29b in Threading::Thread::start()::$_0::__invoke(void*) /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:80:9
    #40 0x5c9d3ae0f6ae in asan_thread_start(void*) crtstuff.c

previously allocated by thread T0 here:
    #0 0x5c9d3ae11a32 in malloc (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x1c9a32) (BuildId: 6d8cf7d79f6603ca)
    #1 0x7a5cba7ac48d in AK::Detail::StringData::create_uninitialized(unsigned long, unsigned char*&) /home/kling/src/serenity/Meta/Lagom/../../AK/StringData.h:19:22
    #2 0x7a5cba7ab4b4 in AK::Detail::StringBase::replace_with_uninitialized_buffer(unsigned long) /home/kling/src/serenity/AK/StringBase.cpp:100:15
    #3 0x7a5cba7a0221 in replace_with_new_string<(lambda at /home/kling/src/serenity/AK/String.cpp:35:5)> /home/kling/src/serenity/Meta/Lagom/../../AK/StringBase.h:78:24
    #4 0x7a5cba7a0221 in AK::String::from_utf8(AK::StringView) /home/kling/src/serenity/AK/String.cpp:35:5
    #5 0x7a5cba7b625f in AK::StringBuilder::to_string() const /home/kling/src/serenity/AK/StringBuilder.cpp:156:12
    #6 0x7a5cbad11750 in AK::ErrorOr<AK::String, AK::Error> AK::String::join<AK::StringView, AK::Vector<int, 0ul>>(AK::StringView const&, AK::Vector<int, 0ul> const&, AK::StringView) /home/kling/src/serenity/Meta/Lagom/../../AK/String.h:180:24
    #7 0x7a5cbacd0ada in TLS::parse_name(Crypto::ASN1::Decoder&, AK::Vector<AK::StringView, 0ul>) /home/kling/src/serenity/Userland/Libraries/LibTLS/Certificate.cpp:286:42
    #8 0x7a5cbacc0d8f in TLS::parse_tbs_certificate(Crypto::ASN1::Decoder&, AK::Vector<AK::StringView, 0ul>) /home/kling/src/serenity/Userland/Libraries/LibTLS/Certificate.cpp:748:27
    #9 0x7a5cbacb9a1b in TLS::Certificate::parse_certificate(AK::Span<unsigned char const>, bool) /home/kling/src/serenity/Userland/Libraries/LibTLS/Certificate.cpp:815:31
    #10 0x7a5cbadc707d in TLS::DefaultRootCACertificates::parse_pem_root_certificate_authorities(AK::Detail::ByteBuffer<32ul>&) /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:610:35
    #11 0x7a5cbadc5af3 in TLS::DefaultRootCACertificates::load_certificates(AK::Span<AK::ByteString>) /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:600:12
    #12 0x7a5cbadc3c31 in TLS::DefaultRootCACertificates::DefaultRootCACertificates() /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:561:24
    #13 0x7a5cbadc2d85 in TLS::DefaultRootCACertificates::the() /home/kling/src/serenity/Userland/Libraries/LibTLS/TLSv12.cpp:572:38
    #14 0x5c9d3ae524c4 in serenity_main(Main::Arguments) /home/kling/src/serenity/Ladybird/RequestServer/main.cpp:53:36
    #15 0x5c9d3af5f88b in main /home/kling/src/serenity/Userland/Libraries/LibMain/Main.cpp:39:19
    #16 0x7a5cba02a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #17 0x7a5cba02a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #18 0x5c9d3ad73534 in _start (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x12b534) (BuildId: 6d8cf7d79f6603ca)

Thread T39 created by T0 here:
    #0 0x5c9d3adf7351 in pthread_create (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x1af351) (BuildId: 6d8cf7d79f6603ca)
    #1 0x7a5cbb6daaf7 in Threading::Thread::start() /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:76:14
    #2 0x5c9d3aecaccb in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::initialize_workers(unsigned long) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:129:21
    #3 0x5c9d3ae61ce2 in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::ThreadPool(AK::Function<void (AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>)>, AK::Optional<unsigned long>) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:72:9
    #4 0x5c9d3ae55b04 in RequestServer::ConnectionFromClient::ConnectionFromClient(AK::NonnullOwnPtr<Core::LocalSocket>) /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:29:7
    #5 0x5c9d3ae5857a in RequestServer::ConnectionFromClient::connect_new_client() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:180:34
    #6 0x5c9d3ae58add in non-virtual thunk to RequestServer::ConnectionFromClient::connect_new_client() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp
    #7 0x5c9d3ae66801 in RequestServerStub::handle(IPC::Message const&) /home/kling/src/serenity/Build/lagom/Userland/Services/RequestServer/RequestServerEndpoint.h:1637:29
    #8 0x7a5cbb7a34e0 in IPC::ConnectionBase::handle_messages() /home/kling/src/serenity/Userland/Libraries/LibIPC/Connection.cpp:77:48
    #9 0x7a5cba956ecf in AK::Function<void ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #10 0x7a5cba9c0f7d in Core::ThreadEventQueue::process() /home/kling/src/serenity/Userland/Libraries/LibCore/ThreadEventQueue.cpp:110:17
    #11 0x7a5cba93b771 in Core::EventLoopImplementationUnix::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp:299:9
    #12 0x7a5cba934ea1 in Core::EventLoop::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoop.cpp:88:20
    #13 0x5c9d3ae525b3 in serenity_main(Main::Arguments) /home/kling/src/serenity/Ladybird/RequestServer/main.cpp:68:23
    #14 0x5c9d3af5f88b in main /home/kling/src/serenity/Userland/Libraries/LibMain/Main.cpp:39:19
    #15 0x7a5cba02a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #16 0x7a5cba02a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #17 0x5c9d3ad73534 in _start (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x12b534) (BuildId: 6d8cf7d79f6603ca)

Thread T35 created by T0 here:
    #0 0x5c9d3adf7351 in pthread_create (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x1af351) (BuildId: 6d8cf7d79f6603ca)
    #1 0x7a5cbb6daaf7 in Threading::Thread::start() /home/kling/src/serenity/Userland/Libraries/LibThreading/Thread.cpp:76:14
    #2 0x5c9d3aecaccb in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::initialize_workers(unsigned long) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:129:21
    #3 0x5c9d3ae61ce2 in Threading::ThreadPool<AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>, RequestServer::ConnectionFromClient::Looper>::ThreadPool(AK::Function<void (AK::Variant<RequestServer::ConnectionFromClient::StartRequest, RequestServer::ConnectionFromClient::EnsureConnection, AK::Empty>)>, AK::Optional<unsigned long>) /home/kling/src/serenity/Meta/Lagom/../../Userland/Libraries/LibThreading/ThreadPool.h:72:9
    #4 0x5c9d3ae55b04 in RequestServer::ConnectionFromClient::ConnectionFromClient(AK::NonnullOwnPtr<Core::LocalSocket>) /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:29:7
    #5 0x5c9d3ae5857a in RequestServer::ConnectionFromClient::connect_new_client() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp:180:34
    #6 0x5c9d3ae58add in non-virtual thunk to RequestServer::ConnectionFromClient::connect_new_client() /home/kling/src/serenity/Userland/Services/RequestServer/ConnectionFromClient.cpp
    #7 0x5c9d3ae66801 in RequestServerStub::handle(IPC::Message const&) /home/kling/src/serenity/Build/lagom/Userland/Services/RequestServer/RequestServerEndpoint.h:1637:29
    #8 0x7a5cbb7a34e0 in IPC::ConnectionBase::handle_messages() /home/kling/src/serenity/Userland/Libraries/LibIPC/Connection.cpp:77:48
    #9 0x7a5cba956ecf in AK::Function<void ()>::operator()() const /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:125:25
    #10 0x7a5cba9c0f7d in Core::ThreadEventQueue::process() /home/kling/src/serenity/Userland/Libraries/LibCore/ThreadEventQueue.cpp:110:17
    #11 0x7a5cba93b771 in Core::EventLoopImplementationUnix::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoopImplementationUnix.cpp:299:9
    #12 0x7a5cba934ea1 in Core::EventLoop::exec() /home/kling/src/serenity/Userland/Libraries/LibCore/EventLoop.cpp:88:20
    #13 0x5c9d3ae525b3 in serenity_main(Main::Arguments) /home/kling/src/serenity/Ladybird/RequestServer/main.cpp:68:23
    #14 0x5c9d3af5f88b in main /home/kling/src/serenity/Userland/Libraries/LibMain/Main.cpp:39:19
    #15 0x7a5cba02a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #16 0x7a5cba02a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #17 0x5c9d3ad73534 in _start (/home/kling/src/serenity/Build/lagom/libexec/RequestServer+0x12b534) (BuildId: 6d8cf7d79f6603ca)

@awesomekling
Copy link
Member

Another one:

VERIFICATION FAILED: !m_socket at /home/kling/src/serenity/Userland/Libraries/LibHTTP/Job.cpp:101
VERIFICATION FAILED: !m_socket at /home/kling/src/serenity/Userland/Libraries/LibHTTP/Job.cpp:101
==3346530==ERROR: AddressSanitizer: attempting double-free on 0x5030000b34a0 in thread T47:
VERIFICATION FAILED: may_defer || !called_from_inside_function at /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:235
VERIFICATION FAILED: may_defer || !called_from_inside_function at /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:235
VERIFICATION FAILED: may_defer || !called_from_inside_function at /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:235
VERIFICATION FAILED: may_defer || !called_from_inside_function at /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:235
VERIFICATION FAILED: may_defer || !called_from_inside_function at /home/kling/src/serenity/Meta/Lagom/../../AK/Function.h:235
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(___interceptor_backtrace+0x79) [0x63acb6c61919]
/home/kling/src/serenity/Build/lagom/lib/liblagom-ak.so.0(ak_verification_failed+0x399) [0x79b0136ff111]
/home/kling/src/serenity/Build/lagom/lib/liblagom-http.so.0 HTTP::Job::start(Core::BufferedSocketBase&) 0x332) [0x79b0148148d2]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x248f6a) [0x63acb6d38f6a]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x30cea3) [0x63acb6dfcea3]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0(+0x156ed0) [0x79b013956ed0]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::ThreadEventQueue::process() 0x98e) [0x79b0139c0f7e]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::EventLoopImplementationUnix::exec() 0x102) [0x79b01393b772]
/home/kling/src/serenity/Build/lagom/lib/liblagom-core.so.0 Core::EventLoop::exec() 0x152) [0x79b013934ea2]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x284af2) [0x63acb6d74af2]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x284592) [0x63acb6d74592]
/home/kling/src/serenity/Build/lagom/lib/liblagom-threading.so.0(+0x15050) [0x79b0146d8050]
/home/kling/src/serenity/Build/lagom/lib/liblagom-threading.so.0(+0x1429c) [0x79b0146d729c]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(+0x1c76af) [0x63acb6cb76af]
/lib/x86_64-linux-gnu/libc.so.6(+0x9ca94) [0x79b01309ca94]
/lib/x86_64-linux-gnu/libc.so.6(+0x129c3c) [0x79b013129c3c]
/home/kling/src/serenity/Build/lagom/libexec/RequestServer(___interceptor_backtrace+0x79) [0x63acb6c61919]
/home/kling/src/serenity/Build/lagom/lib/liblagom-ak.so.0(ak_verification_failed+0x399) [0x79b0136ff111]

@alimpfard alimpfard force-pushed the rs-wao branch 3 times, most recently from f6fb60c to 32a5283 Compare May 16, 2024 15:42
@alimpfard alimpfard marked this pull request as draft May 16, 2024 18:44
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 16, 2024
@alimpfard
LibCore: Add Core::deferred_invoke_if(F, Condition)
526b23f 
This will invoke the function F only if the provided Condition is met,
otherwise the execution of the function F will be further deferred.
@alimpfard
LibTLS: Clear connected/error callbacks before leaving connect()
0fcae10 
Otherwise error events will call into our setup code, eventually leading
to a UAF.
@alimpfard
LibThreading: Add RWLock and RWLockedProtected
bba9dfe
@alimpfard
LibCore: Make Timers and Notifiers aware of threads
5da5e06 
Previously sharing a Timer/Notifier between threads (or just handing
its ownership to another thread) lead to a crash as they are
thread-specific.
This commit makes it so we can handle mutation (i.e. just deletion
or unregistering) in a thread-safe and lockfree manner.
@alimpfard
LibCore: Remove the spammy "too late for a reloading timer" debug log
08a896a
@alimpfard
LibThreading: Expose the ProtectedType alias
183c871
@alimpfard
LibThreading: Add a ThreadPool
0a56e10
@alimpfard alimpfard marked this pull request as ready for review May 16, 2024 19:50
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label May 16, 2024
@DanShaders DanShaders mentioned this pull request May 18, 2024
Open
@alimpfard
RequestServer: Handle IPC requests on multiple threads concurrently
a03a584 
Previously RS handled all the requests in an event loop, leading to
issues with connections being started in the middle of other connections
being started (and potentially blowing up the stack), ultimately causing
requests to be delayed because of other requests.
This commit reworks the way we handle these (specifically starting
connections) by first serialising the requests, and then performing them
in multiple threads concurrently; which yields a significant loading
performance and reliability increase.
@alimpfard
LibCrypto: Move some data around earlier in GHash to make it go faster
1b4d366 
This makes galois_multiply() about 10% faster.
@alimpfard
RequestServer: Serialise accesses to IPC::Connection
d379508 
Most of IPC::Connection is thread-safe, but the responsiveness timer is
very much not so, this commit makes sure only one thread can send stuff
through IPC to avoid having threads racing in IPC::Connection.
This is simply less work than making sure everything IPC::Connection
uses (now and later) is thread-safe.
@awesomekling awesomekling merged commit ddc6222 into SerenityOS:master May 20, 2024
11 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zaggy1024 Zaggy1024 Zaggy1024 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alimpfard @BuggieBot @awesomekling @Zaggy1024