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
            Ryan Ryan created issue -
            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 .
            marko Marko Mäkelä made changes -
            Field Original Value New Value
            Assignee Marko Mäkelä [ marko ]
            Status Open [ 1 ] Needs Feedback [ 10501 ]
            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ä made changes -
            Status Needs Feedback [ 10501 ] Open [ 1 ]
            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.
            marko Marko Mäkelä made changes -
            Fix Version/s 10.3.36 [ 27513 ]
            Fix Version/s 10.4.26 [ 27511 ]
            Fix Version/s 10.5.17 [ 27509 ]
            Fix Version/s 10.6.9 [ 27507 ]
            Fix Version/s 10.7.5 [ 27505 ]
            Fix Version/s 10.8.4 [ 27503 ]
            Fix Version/s 10.9.2 [ 27115 ]
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Closed [ 6 ]
            wlad Vladislav Vaintroub made changes -
            Resolution Fixed [ 1 ]
            Status Closed [ 6 ] Stalled [ 10000 ]
            wlad Vladislav Vaintroub made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            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
            wlad Vladislav Vaintroub made changes -
            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 .
            marko Marko Mäkelä made changes -
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]

            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.