Last 12 weeks · 8 commits
4 of 6 standards met
We (slack) have been seeing a very slow trickle of segfaults from code paths in the cleanup timer in our production environment. This issue is not new, it's been occurring for a while. I do not yet have a repro for these segfaults. Relevant version of squangle we are running: There are two unique stack traces we see. The first is more frequent and appears to occur on a call to std::unordered_map::erase(): 0:string"raise at ../sysdeps/unix/sysv/linux/raise.c:51" 1:string"HPHP::bt_handler at /build/hhvm/hphp/runtime/base/crash-reporter.cpp:270" 2:string"facebook::common::mysql_client::AsyncMysqlClient::activeConnectionRemoved at /build/hhvm/third-party/squangle/src/squangle/mysql_client/AsyncMysqlClient.h:357" 3:string"facebook::common::mysql_client::MysqlConnectionHolder::~MysqlConnectionHolder at /build/hhvm/third-party/squangle/squangle/mysql_client/Connection.cpp:63" 4:string"facebook::common::mysql_client::MysqlConnectionHolder::~MysqlConnectionHolder at /build/hhvm/third-party/squangle/squangle/mysql_client/Connection.cpp:64" 5:string"std::default_delete::operator() at /usr/include/c++/7/bits/unique_ptr.h:78" 6:string"std::unique_ptr::operator() at /build/hhvm/third-party/squangle/squangle/mysql_client/AsyncMysqlClient.cpp:80" 17:string"std::__invoke_impl at /usr/include/c++/7/bits/invoke.h:95" 19:string"std::thread::_Invoker at /usr/include/c++/7/thread:234" 20:string"std::thread::_Invoker at /usr/include/c++/7/thread:243" 21:string"std::thread::_State_impl at /usr/include/c++/7/thread:186" 22:string"start_thread at pthread_create.c:463" 23:string"clone at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95" new AsyncMysqlConnectionPool(darray[ 'per_key_connection_limit' => 20, 'idle_timeout_micros' => 4000000, 'expiration_policy' => "IdleTime", ]); ```
What now actually erases the operation from the wait list when it is found, matching its documented behavior ("Searches for an operation in the queue and removes it"). Previously it only returned whether the operation was present. Why Two call sites depend on the removal side effect: (SyncConnectionPool.cpp) — on sync-wait timeout the operation must be dequeued before runs. With the bug the timed-out operation stayed queued, and the branch that waits for another thread to fulfill the operation could effectively never trigger. (ConnectionPool.h) — the throttling callback dequeues an operation that has just been marked failed; its comment states no other thread could have dequeued it yet, which only makes sense if dequeue actually removes. Without the erase, done/timed-out operations remain in until or a subsequent discards them. In the meantime is inflated, feeding and potentially delaying new connections. Testing Change is self-contained in . Erase is safe: it runs while holding the pool-data lock (or on the single client thread for the sync pool), and the operation is already marked failed/done at both call sites, so no other path can hold a reference that requires it to stay queued. Minimal behavioral change; no new dependencies.
It would be useful for userland Hack code in Slack to determine whether a connection obtained via an HHVM async_mysql connection pool was recycled from the pool or freshly opened, to allow more granular instrumentation than what pool-level stats currently provide. See https://github.com/facebook/hhvm/issues/8013 for the full rationale. Add two new connection fields that expose this information. A followup diff will expose these to HHVM/Hack.
Squangle as included in HHVM OSS currently doesn't compile. Apply fixes as needed to make the OSS build functional again. Use a new SQUANGLE_OSS compile definition for OSS-specific code and to conditionally include Meta-internal headers. Introduce OSS-specific shims for OdsCounterHelper and checked_memcpy(), which are internal. Use fmt::underlying() to format enums that do not have a corresponding formatter to avoid errors on debug builds with newer fmtlib versions. Move Query constructors from the header to the implementation to avoid pointer arithmetic with an incomplete definition of QueryArgument when populating .
Repository: facebook/squangle. Description: SQuangLe is a C++ API for accessing MySQL servers Stars: 126, Forks: 57. Primary language: C++. Languages: C++ (99.9%), C (0.1%). Open PRs: 0, open issues: 2. Last activity: 13h ago. Community health: 75%. Top contributors: jkedgar, abal147, aditya-jalan, drilibo, jrahman-zz, yfeldblum, sarangbh, maxgeorg, r-barnes, jtwarren and others.