Boost C++ Libraries: Ticket #7720: exception lock_error while intensive locking/unlocking of mutex

owner, status, description changed

viboes — Fri, 23 Nov 2012 06:36:00 GMT

owner changed from Anthony Williams to viboes
status new → assigned
description modified (diff)

Please, could you try defining BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN to see if the error persist on the generic implementation?

sergey.stepanov@… — Fri, 23 Nov 2012 11:08:57 GMT

Replying to viboes: With this macro it works fine.

viboes — Sun, 25 Nov 2012 08:18:14 GMT

Replying to sergey.stepanov@…:

Replying to viboes: With this macro it works fine.

The specific implementation on Windows had some bugs that were fixed by the generic implementation. This is why I have added this macro. It will be defined by default since 1.53. Could we close this ticket?

anonymous — Mon, 26 Nov 2012 07:45:06 GMT

Replying to viboes:

The specific implementation on Windows had some bugs that were fixed by the generic implementation. This is why I have added this macro. It will be defined by default since 1.53. Could we close this ticket?

I think we could. Thank you for your help. Sergey.

status changed; resolution set; milestone deleted

viboes — Mon, 26 Nov 2012 23:11:51 GMT

status assigned → closed
resolution → wontfix
milestone To Be Determined

status changed; milestone set; resolution deleted

viboes — Sat, 19 Jan 2013 10:43:09 GMT

status closed → reopened
resolution wontfix
milestone → To Be Determined

Reopened as BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN will not be defined by default and as #7906 states there is a lost in performances when this define is defined.

Tue, 12 Feb 2013 05:05:55 GMT

Ooops, the same issue in my second project already. boost 1.52. Any plans when fix for upgrade_lock will be available?

viboes — Tue, 12 Feb 2013 06:25:05 GMT

Replying to Andrey <nikolay@…>:

Ooops, the same issue in my second project already. boost 1.52. Any plans when fix for upgrade_lock will be available?

No. For the time been I have no idea how to fix it.

Until the share_mutex errors have been fixed, please define BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN. Even if it takes more time it works correctly (well better).

Tue, 12 Feb 2013 06:53:13 GMT

Unfortunately, my first project where the problem is reproducible is really time critical module. It is a reason why the performance issue with generic implementation has been found in such little time period. I have made some tests on 1.52 and found that from the performance perspective it is better for me don't use upgrade_lock at all than use generic implementation of shared_mutex. Now while new release of my program is under development I am waiting for the fix for this issue. But if it will not be fixed till end of March I will need to choose:

downgrade boost to 1.44 to have fast and bug-free upgrade_lock with shared_mutex

rewrite code to don't use upgrade_lock in my projects

severity changed

viboes — Wed, 13 Feb 2013 07:19:49 GMT

severity Problem → Regression

Replying to Andrey <nikolay@…>:

Unfortunately, my first project where the problem is reproducible is really time critical module. It is a reason why the performance issue with generic implementation has been found in such little time period. I have made some tests on 1.52 and found that from the performance perspective it is better for me don't use upgrade_lock at all than use generic implementation of shared_mutex. Now while new release of my program is under development I am waiting for the fix for this issue. But if it will not be fixed till end of March I will need to choose:

downgrade boost to 1.44 to have fast and bug-free upgrade_lock with shared_mutex

This is an interesting information. BTW, have you tried 1.45? Do you know the first version which introduced the regression?

or

rewrite code to don't use upgrade_lock in my projects

I can not ensure you when this will be fixed but the fact this is a regression would surely help.

Wed, 13 Feb 2013 08:44:53 GMT

Replying to viboes:

This is an interesting information. BTW, have you tried 1.45? Do you know the first version which introduced the regression?

I haven't tried versions 1.45-1.51. The previous boost version I used is 1.44. May be author of this ticket knows first boost version affected.

sergey.stepanov@… — Wed, 13 Feb 2013 09:07:02 GMT

Replying to Andrey <nikolay@…>:

I haven't tried versions 1.45-1.51. The previous boost version I used is 1.44. May be author of this ticket knows first boost version affected.

First time I encountered this problem is version 1.51. Previous was 1.45.

viboes — Wed, 13 Feb 2013 11:59:13 GMT

Replying to sergey.stepanov@…:

Replying to Andrey <nikolay@…>:

I haven't tried versions 1.45-1.51. The previous boost version I used is 1.44. May be author of this ticket knows first boost version affected.

First time I encountered this problem is version 1.51. Previous was 1.45.

Do you mean that 1.45 works? If the answer is yes, I should be the responsable of the regression :(

I will take a deeper look.

sergey.stepanov@… — Wed, 13 Feb 2013 12:12:27 GMT

Do you mean that 1.45 works?

I'm not sure. But it is not difficult to verify. Just run my example and wait a few minutes.

viboes — Sat, 16 Feb 2013 17:29:37 GMT

Hi,

I didn't had access to a windows machine during the week. I have tested it now I confirm the error.

The following patch works for me. Could you check it?

svn diff boost/thread/win32/shared_mutex.hpp
Index: boost/thread/win32/shared_mutex.hpp
===================================================================
--- boost/thread/win32/shared_mutex.hpp	(revision 82919)
+++ boost/thread/win32/shared_mutex.hpp	(working copy)
@@ -737,7 +737,7 @@
                     {
                         release_waiters(old_state);
                     } else {
-                        release_waiters(old_state);
+                        //release_waiters(old_state);
                     }
                     break;
                 }

milestone changed

viboes — Sat, 16 Feb 2013 23:24:25 GMT

milestone To Be Determined → Boost 1.54.0

sergey.stepanov@… — Mon, 18 Feb 2013 08:08:21 GMT

Replying to viboes: With this correction there are no exceptions. Used: boost ver. 1.51, MS VS 2005, MS Windows 7 Pro SP1.

viboes — Mon, 18 Feb 2013 12:12:04 GMT

Committed in trunk revision [82973].

status changed; resolution set

viboes — Sat, 23 Mar 2013 02:11:23 GMT

status reopened → closed
resolution → fixed

Committed revision [83525].

ajay.sonawane@… — Wed, 05 Nov 2014 05:23:18 GMT

I modified the test program slightly and I got the lock_Error exceptions again. I'm using boost 1.54.0. Please let me know if the fix for this problem is available.

shared_mutex mtx;

unsigned int sharedVariable = 0;

void upgrade() {

while (true) {

try {

upgrade_lock<shared_mutex> lock(mtx); std::cout << sharedVariable;

} catch(boost::lock_error& err) {

std::cout << "Exception in Upgrade: " << err.what() << std::endl;

}

}

}

void shared() {

while (true) {

try {

shared_lock<shared_mutex> lock(mtx); std::cout << sharedVariable;

} catch(boost::lock_error& err) {

std::cout << "Exception in Shared: " << err.what() << std::endl;

}

}

}

void unique() {

while (true) {

try {

unique_lock<shared_mutex> lock(mtx); ++sharedVariable;;

} catch(boost::lock_error& err) {

std::cout << "Exception in Unique: " << err.what() << std::endl;

}

}

}

int main() {

unsigned int numThreads = 1000;

boost::thread_group grp; {

for (unsigned int i = 0; i < numThreads; i++) {

if(i % 5 == 0)

grp.create_thread (boost::bind (&unique));

else if(i%3 == 0)

grp.create_thread (boost::bind (&upgrade));

else

grp.create_thread (boost::bind (&shared));

}

}

grp.join_all ();

return 0;

}

viboes — Wed, 05 Nov 2014 06:44:36 GMT

Sorry for not updating the ticket. This has been fixed as

        void release_shared_waiters(state_data old_state)
        {
            if(old_state.shared_waiting || old_state.exclusive_waiting)
            {
                BOOST_VERIFY(detail::win32::ReleaseSemaphore(semaphores[unlock_sem],old_state.shared_waiting + (old_state.exclusive_waiting?1:0),0)!=0);
            }
        }
...
                    if(last_reader)
                    {
                        release_waiters(old_state);
                    }
                    else
                    {
                        release_shared_waiters(old_state);
                    }

Please, could you try it?

ajay.sonawane@… — Wed, 05 Nov 2014 07:35:42 GMT

Which is the boost version in which it is fixed ?

viboes — Wed, 05 Nov 2014 18:30:13 GMT

It was included with https://github.com/boostorg/thread/commit/6f91af2154da808eb29de2e9e12c3e1774cdea31

Try to address #9569. 5 Oct

This has been delivered with Boost 1.57.

ajay.sonawane@… — Thu, 06 Nov 2014 04:22:22 GMT

I did same changed in boost/thread/win32/shared_mutex.hpp from boost 1.54.0 and compiled my program. I presume I do not need to make recompile boost library. After the above changes you mentioned, I'm still getting boost::lock_error - Operation completed successfully. I guess I may need to download boost 1.57 to verify this issue.

m@… — Wed, 25 Mar 2015 01:08:43 GMT

I am seeing this error on boost 1.57. It occurs with heavy mixed use of shared_lock, upgrade_lock, and upgrade_to_unique_lock on a shared_mutex. My project is on Windows 64-bit.

status changed; resolution deleted

duchungjava@… — Thu, 17 Sep 2015 08:00:41 GMT

status closed → reopened
resolution fixed

To whom it may concern,

On Boost 1.57, I can replicate this problem with the following code:

boost::shared_mutex mutex;
void shared_proc() {
  while ( 1 ) {
    boost::shared_lock<boost::shared_mutex> lock(mutex);
    boost::this_thread::sleep_for(boost::chrono::seconds(2));
  }
}
void upgrade_proc() {
  while ( 1 ) {
    boost::upgrade_lock<boost::shared_mutex> lock(mutex);
    boost::this_thread::sleep_for(boost::chrono::milliseconds(1));
  }
}
int main() {
  boost::thread ts(boost::bind(shared_proc));
  boost::thread t1(boost::bind(upgrade_proc));
  boost::thread t2(boost::bind(upgrade_proc));
  ts.join();
  t1.join();
  t2.join();
  return 0;
}

By testing this code with different Boost versions, I've confirmed that:

Boost 1.56 doesn't throw exception
Boost 1.57 and 1.59 throw exception instantly

I think the root reason is: The variable state.shared_count in shared_mutex.hpp couldn't be decreased/reset properly when a shared_mutex is being used for both shared_lock and upgrade_lock.

(I'm using Visual Studio 2012 in this test case)

Yours sincerely, Hung Mai

anonymous — Thu, 17 Sep 2015 08:15:11 GMT

I'm sorry that I made a mistake in the previous post.

This is the updated conclusion: "I think the variable state.shared_waiting in shared_mutex.hpp couldn't be decreased/reset properly when a shared_mutex is being used for both shared_lock and upgrade_lock"

Thank you,

Hung Mai

version, milestone changed

duchungjava@… — Thu, 17 Sep 2015 08:50:56 GMT

version Boost 1.51.0 → Boost 1.59.0
milestone Boost 1.54.0 → To Be Determined

viboes — Thu, 17 Sep 2015 17:53:24 GMT

I'm a little bit busy for two weeks.

For those that can not wait until we have a fix,

you could you define BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN. It seems that this works well even if it performs worst.

duchungjava@… — Fri, 18 Sep 2015 10:19:45 GMT

Thank you Viboes,

Yes, it's true that BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN works well in all of my test cases. However, due to the performance issue, it's a difficult decision for us to apply it to our system.

In one of my experiments, on Boost 1.59, if I modify the unlock_upgrade() to reset "new_state.shared_waiting" to zero regardless what the status of "last_reader" is, everything seems to work properly.

I hope you will consider this experiment when you have time.

Best regards, Hung Mai

viboes — Fri, 18 Sep 2015 21:37:39 GMT

Please, be free to provide a patch. If it works for you I will commit it.

attachment set

Mon, 21 Sep 2015 08:42:50 GMT

attachment → shared_mutex.hpp

Starting from 1.57, unlock_upgrade() should always reset the variable "new_state.shared_waiting"

Mon, 21 Sep 2015 08:57:29 GMT

Hi Viboes,

Since I don't know how to provide the patch in the desired format, I've attached the file with the changes I made based on Boost-1.59

Basically, there is just one small change in the unlock_upgrade function as following:

void unlock_upgrade()
{
  // ...
  new_state.shared_waiting=0; // adds this line
  if(last_reader)
  {
    if(new_state.exclusive_waiting)
    {
      --new_state.exclusive_waiting;
      new_state.exclusive_waiting_blocked=false;
    }
    // new_state.shared_waiting=0; // comment out this line
  }
  // ...
}

Thank you,

Hung Mai

version, milestone changed

viboes — Wed, 23 Sep 2015 22:09:24 GMT

version Boost 1.59.0 → Boost 1.51.0
milestone To Be Determined → Boost 1.60.0

I have committed your patch

https://github.com/boostorg/thread/commit/205a1d7e2b7ca12058ea94fb147d27cdaad2753a

I will check if there are no regressions on the Boost testers.

Thanks for identifying this.

Thu, 24 Sep 2015 01:55:00 GMT

Thank you Viboes,

It's my pleasure to contribute to the community.
I hope everything will go smoothly.

Best regards,

status changed; resolution set

viboes — Sun, 27 Sep 2015 13:40:56 GMT

status reopened → closed
resolution → fixed

https://github.com/boostorg/thread/commit/730cb550e6d969520c3f39e0ddf5d80351bddf3a

anonymous — Thu, 28 Jul 2016 11:44:49 GMT

Hi,

I'm hitting this bug, and I'm stuck with Boost 1.55 for the time being. Is there a workaround that doesn't require modifying Boost's source code? Some posts of this thread mention BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN: how bad is the performance loss?

Thanks for your help.

Franz