Opened 10 years ago

Closed 7 years ago

Last modified 6 years ago

#7720 closed Bugs (fixed)

exception lock_error while intensive locking/unlocking of mutex

Reported by: sergey.stepanov@… Owned by: viboes
Milestone: Boost 1.60.0 Component: thread
Version: Boost 1.51.0 Severity: Regression
Keywords: lock_error Cc:

Description (last modified by viboes)

Hi, Anthony and Vicente.

////////////////////////////////////////////
#include <boost/thread/thread.hpp>
using namespace boost;

shared_mutex mtx;

void f()
{
    while (true)
    {
        upgrade_lock<shared_mutex> lock(mtx);
    }
}

void g()
{
    while (true)
    {
        shared_lock<shared_mutex> lock(mtx);
    }
}

void h()
{
    while (true)
    {
        unique_lock<shared_mutex> lock(mtx);
    }
}

int main()
{
    thread t0(f);
    thread t1(g);
    thread t2(h);

    t0.join();
    t1.join();
    t2.join();

    return 0;
}
////////////////////////////////////////////

I ran this program and got exception boost::lock_error in some minutes.

Used MS VS 2005 Version 8.0.50727.867, Boost C++ Libraries 1.51 and MS Windows 7 Pro version 6.1.7601 SP1.

Regards, Sergey.

Attachments (1)

shared_mutex.hpp (31.0 KB ) - added by Hung Mai <duchungjava@…> 7 years ago.
Starting from 1.57, unlock_upgrade() should always reset the variable "new_state.shared_waiting"

Download all attachments as: .zip

Change History (37)

comment:1 by viboes, 10 years ago

Description: modified (diff)
Owner: changed from Anthony Williams to viboes
Status: newassigned

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

in reply to:  1 ; comment:2 by sergey.stepanov@…, 10 years ago

Replying to viboes: With this macro it works fine.

in reply to:  2 ; comment:3 by viboes, 10 years ago

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?

in reply to:  3 comment:4 by anonymous, 10 years ago

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.

comment:5 by viboes, 10 years ago

Milestone: To Be Determined
Resolution: wontfix
Status: assignedclosed

comment:6 by viboes, 10 years ago

Milestone: To Be Determined
Resolution: wontfix
Status: closedreopened

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.

comment:7 by Andrey <nikolay@…>, 10 years ago

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

in reply to:  7 comment:8 by viboes, 10 years ago

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).

comment:9 by Andrey <nikolay@…>, 10 years ago

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

or

  • rewrite code to don't use upgrade_lock in my projects

in reply to:  9 ; comment:10 by viboes, 10 years ago

Severity: ProblemRegression

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.

in reply to:  10 ; comment:11 by Andrey <nikolay@…>, 10 years ago

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.

in reply to:  11 ; comment:12 by sergey.stepanov@…, 10 years ago

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.

in reply to:  12 ; comment:13 by viboes, 10 years ago

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.

in reply to:  13 comment:14 by sergey.stepanov@…, 10 years ago

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.

comment:15 by viboes, 10 years ago

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;
                 }

comment:16 by viboes, 10 years ago

Milestone: To Be DeterminedBoost 1.54.0

in reply to:  16 comment:17 by sergey.stepanov@…, 10 years ago

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

comment:18 by viboes, 10 years ago

Committed in trunk revision [82973].

comment:19 by viboes, 10 years ago

Resolution: fixed
Status: reopenedclosed

Committed revision [83525].

comment:20 by ajay.sonawane@…, 8 years ago

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;

}

comment:21 by viboes, 8 years ago

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?

comment:22 by ajay.sonawane@…, 8 years ago

Which is the boost version in which it is fixed ?

comment:23 by viboes, 8 years ago

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.

comment:24 by ajay.sonawane@…, 8 years ago

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.

comment:25 by m@…, 8 years ago

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.

comment:26 by duchungjava@…, 7 years ago

Resolution: fixed
Status: closedreopened

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:

  1. Boost 1.56 doesn't throw exception
  2. 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

comment:27 by anonymous, 7 years ago

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

comment:28 by duchungjava@…, 7 years ago

Milestone: Boost 1.54.0To Be Determined
Version: Boost 1.51.0Boost 1.59.0

comment:29 by viboes, 7 years ago

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.

comment:30 by duchungjava@…, 7 years ago

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

comment:31 by viboes, 7 years ago

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

by Hung Mai <duchungjava@…>, 7 years ago

Attachment: shared_mutex.hpp added

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

comment:32 by Hung Mai <duchungjava@…>, 7 years ago

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

comment:33 by viboes, 7 years ago

Milestone: To Be DeterminedBoost 1.60.0
Version: Boost 1.59.0Boost 1.51.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.

Last edited 7 years ago by viboes (previous) (diff)

comment:34 by Hung Mai <duchungjava@…>, 7 years ago

Thank you Viboes,

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

Best regards,

comment:36 by anonymous, 6 years ago

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

Note: See TracTickets for help on using tickets.