Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-26271

Crashes in net_field_length since commit 295f3e4cfb4a8f132f36d53475efc92f2487aa97

Details

    • Bug
    • Status: Closed (View Workflow)
    • Major
    • Resolution: Not a Bug
    • 10.6.3
    • N/A
    • libmariadb
    • Linux x86_64

    Description

      Since commit 295f3e4cfb4a8f132f, mariadb crashes (and has severe performance issues) when scanning the DigiKam database. The initial scan (with ~5000 pics) used to take <1 second with MariaDB 10.5, now it takes 10 seconds to reach 10% and then crashes

      (gdb) bt
      #0  net_field_length (packet=packet@entry=0x7fffffffd1c8) at /usr/src/debug/server/libmariadb/libmariadb/mariadb_lib.c:296
      #1  0x00007fff6749d6dc in ps_fetch_string (r_param=0x55555b44b988, field=<optimized out>, row=0x7fffffffd1c8)
          at /usr/src/debug/server/libmariadb/libmariadb/ma_stmt_codec.c:1195
      #2  0x00007fff67497ca7 in mthd_stmt_fetch_to_bind (stmt=0x55555b3d0a90, row=<optimized out>)
          at /usr/src/debug/server/libmariadb/libmariadb/mariadb_stmt.c:423
      #3  0x00007fff674997b7 in mysql_stmt_fetch (stmt=0x55555b3d0a90) at /usr/src/debug/server/libmariadb/libmariadb/mariadb_stmt.c:1463
      #4  0x00007fff840192ed in  () at /usr/lib/qt/plugins/sqldrivers/libqsqlmysql.so
      #5  0x00007ffff70c1819 in Digikam::BdEngineBackend::readToList(Digikam::DbEngineSqlQuery&) () at /usr/lib/libdigikamcore.so.7.3.0
      #6  0x00007ffff70c1942 in Digikam::BdEngineBackend::handleQueryResult(Digikam::DbEngineSqlQuery&, QList<QVariant>*, QVariant*) ()
          at /usr/lib/libdigikamcore.so.7.3.0
      #7  0x00007ffff70c305b in Digikam::BdEngineBackend::execSql(QString const&, QVariant const&, QList<QVariant>*, QVariant*) ()
          at /usr/lib/libdigikamcore.so.7.3.0
      #8  0x00007ffff756f835 in Digikam::CoreDB::getItemPosition(long long, QFlags<Digikam::DatabaseFields::ItemPositionsField>) const ()
          at /usr/lib/libdigikamdatabase.so.7.3.0
      #9  0x00007ffff75e7312 in  () at /usr/lib/libdigikamdatabase.so.7.3.0
      #10 0x00007ffff75e7a3c in Digikam::ItemPosition::ItemPosition(long long) () at /usr/lib/libdigikamdatabase.so.7.3.0
      #11 0x00007ffff75d011c in Digikam::ItemInfo::imagePosition() const () at /usr/lib/libdigikamdatabase.so.7.3.0
      #12 0x00007ffff75d0432 in Digikam::ItemInfo::hasCoordinates() const () at /usr/lib/libdigikamdatabase.so.7.3.0
      #13 0x00007ffff7d48e2c in  () at /usr/lib/libdigikamgui.so.7.3.0
      #14 0x00007ffff55774ff in QObject::event(QEvent*) () at /usr/lib/libQt5Core.so.5
      #15 0x00007ffff601ed62 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
      #16 0x00007ffff554a3aa in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
      #17 0x00007ffff554d4a9 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/libQt5Core.so.5
      #18 0x00007ffff55a39a4 in  () at /usr/lib/libQt5Core.so.5
      #19 0x00007fffe449210c in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
      --Type <RET> for more, q to quit, c to continue without paging--
      #20 0x00007fffe44e5b99 in  () at /usr/lib/libglib-2.0.so.0
      #21 0x00007fffe448f871 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
      #22 0x00007ffff55a2fd6 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
      #23 0x00007ffff5548d1c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
      #24 0x00007ffff7a3afea in Digikam::ScanController::completeCollectionScanCore(bool, bool) () at /usr/lib/libdigikamgui.so.7.3.0
      #25 0x00007ffff7bcc4e9 in Digikam::NewItemsFinder::slotStart() () at /usr/lib/libdigikamgui.so.7.3.0
      #26 0x00007ffff55774ff in QObject::event(QEvent*) () at /usr/lib/libQt5Core.so.5
      #27 0x00007ffff601ed62 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
      #28 0x00007ffff554a3aa in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
      #29 0x00007ffff554d4a9 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/libQt5Core.so.5
      #30 0x00007ffff55a39a4 in  () at /usr/lib/libQt5Core.so.5
      #31 0x00007fffe449210c in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
      #32 0x00007fffe44e5b99 in  () at /usr/lib/libglib-2.0.so.0
      #33 0x00007fffe448f871 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
      #34 0x00007ffff55a2fd6 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
      #35 0x00007ffff5548d1c in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
      #36 0x00007ffff623325e in QDialog::exec() () at /usr/lib/libQt5Widgets.so.5
      #37 0x00007ffff70498e3 in Digikam::FilesDownloader::startDownload() () at /usr/lib/libdigikamcore.so.7.3.0
      #38 0x000055555555a71b in  ()
      #39 0x00007ffff4eb9b25 in __libc_start_main () at /usr/lib/libc.so.6
      #40 0x000055555555bece in _start ()
      

      Downstream report: https://bugs.kde.org/show_bug.cgi?id=440296

      Attachments

        Issue Links

          Activity

            Testcase:

              MYSQL *mysql = mysql_init(nullptr);
              if ((mysql = mysql_real_connect(mysql, nullptr, nullptr, nullptr, nullptr, 0, "/tmp/mysql.socket", 0))) {
                  MYSQL_STMT *stmt = mysql_stmt_init(mysql);
                  if (mysql_stmt_prepare(stmt, querystr, strlen(querystr)) == 0) {
                      char result[64] = {};
                      unsigned long length = sizeof(result) - 1;
                      my_bool nullIndicator = false;
                      MYSQL_BIND bind = {
                          .length = &length,
                          .is_null = &nullIndicator,
                          .buffer = &result,
                          .buffer_length = sizeof(result),
                          .buffer_type = MYSQL_TYPE_STRING,
                      };
             
                      MYSQL_RES *meta = mysql_stmt_result_metadata(stmt);
                      if (mysql_num_fields(meta) == 1) {
                          MYSQL_FIELD *fieldInfo = mysql_fetch_field(meta);
                          Q_ASSERT(fieldInfo->length < length);
                          bind.buffer_length = length = fieldInfo->length + 1;
                          fieldInfo->type = MYSQL_TYPE_STRING;      // <=== this is the problem
             
                          Q_ASSERT(mysql_fetch_field(meta) == nullptr);
             
                          mysql_stmt_reset(stmt);
                          Q_ASSERT(mysql_stmt_param_count(stmt) == 0);
                          mysql_stmt_execute(stmt);
                          mysql_stmt_bind_result(stmt, &bind);
                          mysql_stmt_store_result(stmt);
                          mysql_stmt_data_seek(stmt, 0);
                          mysql_stmt_fetch(stmt);
                          printf("Result: %.*s\n", length, result);
                      }
                  }
                  mysql_stmt_close(stmt);
                }
            

            thiagomacieira Thiago Macieira added a comment - Testcase: MYSQL *mysql = mysql_init(nullptr); if ((mysql = mysql_real_connect(mysql, nullptr, nullptr, nullptr, nullptr, 0, "/tmp/mysql.socket", 0))) { MYSQL_STMT *stmt = mysql_stmt_init(mysql); if (mysql_stmt_prepare(stmt, querystr, strlen(querystr)) == 0) { char result[64] = {}; unsigned long length = sizeof(result) - 1; my_bool nullIndicator = false; MYSQL_BIND bind = { .length = &length, .is_null = &nullIndicator, .buffer = &result, .buffer_length = sizeof(result), .buffer_type = MYSQL_TYPE_STRING, };   MYSQL_RES *meta = mysql_stmt_result_metadata(stmt); if (mysql_num_fields(meta) == 1) { MYSQL_FIELD *fieldInfo = mysql_fetch_field(meta); Q_ASSERT(fieldInfo->length < length); bind.buffer_length = length = fieldInfo->length + 1; fieldInfo->type = MYSQL_TYPE_STRING; // <=== this is the problem   Q_ASSERT(mysql_fetch_field(meta) == nullptr);   mysql_stmt_reset(stmt); Q_ASSERT(mysql_stmt_param_count(stmt) == 0); mysql_stmt_execute(stmt); mysql_stmt_bind_result(stmt, &bind); mysql_stmt_store_result(stmt); mysql_stmt_data_seek(stmt, 0); mysql_stmt_fetch(stmt); printf("Result: %.*s\n", length, result); } } mysql_stmt_close(stmt); }

            fieldInfo->type = MYSQL_TYPE_STRING;      // <=== this is the problem
            

            Indeed, this is a problem. User of this API is not supposed to change MYSQL_FIELD * returned by mysql_fetch_field. What's the point? It is internally used by the server to know how to decipher the data in prepared statements.

            wlad Vladislav Vaintroub added a comment - fieldInfo->type = MYSQL_TYPE_STRING; // <=== this is the problem Indeed, this is a problem. User of this API is not supposed to change MYSQL_FIELD * returned by mysql_fetch_field. What's the point? It is internally used by the server to know how to decipher the data in prepared statements.
            wlad Vladislav Vaintroub added a comment - - edited

            The "field length" is not the data length used for storage, nor in the client-server protocol, nor in client structs.

            Just like int(10) in "show create table t" does not represent 10-byte integers. It is "display length", the length in characters for textual representation.

            wlad Vladislav Vaintroub added a comment - - edited The "field length" is not the data length used for storage, nor in the client-server protocol, nor in client structs. Just like int(10) in "show create table t" does not represent 10-byte integers. It is "display length", the length in characters for textual representation.

            Indeed, this is a problem. User of this API is not supposed to change MYSQL_FIELD * returned by mysql_fetch_field. What's the point? It is internally used by the server to know how to decipher the data in prepared statements.

            Because the returned pointer wasn't pointing to const data, so whoever wrote the code to support binding results to output variables with the MySQL 4.1.8 API made a shortcut. It happened some time between Qt 3.3 (2004) and 4.5 (2009), for which there is no public Git history, so I can't pinpoint the exact commit. But you can see the oldest implementation at https://github.com/qt/qt/blob/v4.5.1/src/sql/drivers/mysql/qsql_mysql.cpp#L331-L376. So that developer realised that the output type had to be forced to string for some of the column types, they modified fieldInfo->type.

            Not an excuse, but it's likely what happened.

            I recommend adding const to data structures that the user shouldn't modify, when you have a chance to clean the API and apply const-correctness.

            thiagomacieira Thiago Macieira added a comment - Indeed, this is a problem. User of this API is not supposed to change MYSQL_FIELD * returned by mysql_fetch_field. What's the point? It is internally used by the server to know how to decipher the data in prepared statements. Because the returned pointer wasn't pointing to const data, so whoever wrote the code to support binding results to output variables with the MySQL 4.1.8 API made a shortcut. It happened some time between Qt 3.3 (2004) and 4.5 (2009), for which there is no public Git history, so I can't pinpoint the exact commit. But you can see the oldest implementation at https://github.com/qt/qt/blob/v4.5.1/src/sql/drivers/mysql/qsql_mysql.cpp#L331-L376 . So that developer realised that the output type had to be forced to string for some of the column types, they modified fieldInfo->type . Not an excuse, but it's likely what happened. I recommend adding const to data structures that the user shouldn't modify, when you have a chance to clean the API and apply const-correctness.
            wlad Vladislav Vaintroub added a comment - - edited

            I'm closing as not a bug. I can't stop wondering how did that in Qt ever work, it could not have in my opinion. If all Qt wants are strings, there is a wonderful and easy API, mysql_query which gives strings. You do not need prepared statement for that..

            Const is a good suggestion, but I'm not sure 20 year old APIs can be fixed like that.

            wlad Vladislav Vaintroub added a comment - - edited I'm closing as not a bug. I can't stop wondering how did that in Qt ever work, it could not have in my opinion. If all Qt wants are strings, there is a wonderful and easy API, mysql_query which gives strings. You do not need prepared statement for that.. Const is a good suggestion, but I'm not sure 20 year old APIs can be fixed like that.

            People

              georg Georg Richter
              arojas Antonio Rojas
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.