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

Two improper locking bugs in the method compress_write

Details

    Description

      Hi, developers, thank you for your checking. Does it seem the lock ctrl_mutex is not released correctly when writing to the destination stream fails and writing to the destination stream fails in the compress_write?

      This is similar to the lock data_mutex, which is acquired in a nearby place. It is not released when writing to the destination stream fails and writing to the destination stream fails in the compress_write.

      https://github.com/MariaDB/server/blob/10.9/extra/mariabackup/ds_compress.cc#L234-L261

      Thank you for your checking!

      Attachments

        Issue Links

          causes

          Bug - A problem which impairs or prevents the functions of the product. MDEV-29043 mariabackup --compress --parallel=4 hangs in current 10.5.17 from GitHub

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            Thank you for reviewing the code. Do you think that my suggested cleanup related to MDEV-28690 would fix this? It would remove the ctrl_mutex altogether, and simply protect all fields with data_mutex.

            marko Marko Mäkelä added a comment - Thank you for reviewing the code. Do you think that my suggested cleanup related to MDEV-28690 would fix this? It would remove the ctrl_mutex altogether, and simply protect all fields with data_mutex .
            Ryan Ryan added a comment -

            I am not familiar with the code, yet I think the patch could fix the bugs. Thank you very much.

            Ryan Ryan added a comment - I am not familiar with the code, yet I think the patch could fix the bugs. Thank you very much.
            marko Marko Mäkelä added a comment -

            I am not very familiar with this code either, but I think that simpler code is easier to follow.

            marko Marko Mäkelä added a comment - I am not very familiar with this code either, but I think that simpler code is easier to follow.
            wlad Vladislav Vaintroub added a comment -

            Apprently, this broke --compress entirely, and was pushed without any testing.
            It might need to be redone or maybe, we can just ignore the bug since it refers to deprecated --compress interface. I'm going to revert it due to MDEV-29043

            wlad Vladislav Vaintroub added a comment - Apprently, this broke --compress entirely, and was pushed without any testing. It might need to be redone or maybe, we can just ignore the bug since it refers to deprecated --compress interface. I'm going to revert it due to MDEV-29043
            marko Marko Mäkelä added a comment -

            wlad, sorry, as noted in MDEV-29043, I wrongly assumed that the test mariabackup.compress_qpress would be run somewhere on our CI systems. Fixed again.

            marko Marko Mäkelä added a comment - wlad , sorry, as noted in MDEV-29043 , I wrongly assumed that the test mariabackup.compress_qpress would be run somewhere on our CI systems. Fixed again .

            People

              marko Marko Mäkelä
              Ryan Ryan
              Votes:
              0 Vote for this issue
              Watchers:
              3 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.