- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
There is a bug in concurrent_monitor.cpp/h when compiled withTBB_USE_ASSERT=1 TBB_USE_DEBUG=0
There is an assertion in the concurrent_monitor destructor which verify the waitset is empty, but in four places inside this files the waitset is cleared only where TBB_USE_DEBUG is true (look for "temp.clear()").
When compiled with previously mentioned settings, the waitset is not cleared and the assertion in the constructor fails.
I patched the #if's, the minimal change to be able to build, and I'llsubmit the patch though the submision form ASAP.
The waitset is in all the 4 places a temporary object and the clear() is the last instruction before the end of the definition scope, so the #if seems OK, since the clear function only resets value of some internal variables.
void clear() {head.next = &head; head.prev = &head;__TBB_store_relaxed(count, 0);}
There is an assertion in the concurrent_monitor destructor which verify the waitset is empty, but in four places inside this files the waitset is cleared only where TBB_USE_DEBUG is true (look for "temp.clear()").
When compiled with previously mentioned settings, the waitset is not cleared and the assertion in the constructor fails.
I patched the #if's, the minimal change to be able to build, and I'llsubmit the patch though the submision form ASAP.
The waitset is in all the 4 places a temporary object and the clear() is the last instruction before the end of the definition scope, so the #if seems OK, since the clear function only resets value of some internal variables.
void clear() {head.next = &head; head.prev = &head;__TBB_store_relaxed(count, 0);}
Link Copied
7 Replies
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
I guess you meant ~circular_doubly_linked_list_with_sentinel(), instead of ~concurrent_monitor()? That I would confirm, although I only see 3 such uses in concurrent_monitor.{h,cpp}, instead of 4.
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Excuse me, you are right in both comments.
I'm talking about thecircular_doubly_linked_list_with_sentinel destructor not the concurrent_monitor one.
There are only 3 "temp.clear()", the fourth correction is in the "#if !TBB_USE_DEBUG" around the private keyword incircular_doubly_linked_list_with_sentinel which must be changed to "#if !TBB_USE_DEBUG && !TBB_USE_ASSERT".
I'm talking about thecircular_doubly_linked_list_with_sentinel destructor not the concurrent_monitor one.
There are only 3 "temp.clear()", the fourth correction is in the "#if !TBB_USE_DEBUG" around the private keyword incircular_doubly_linked_list_with_sentinel which must be changed to "#if !TBB_USE_DEBUG && !TBB_USE_ASSERT".
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
How about this?
[cpp]#if !TBB_USE_ASSERT private: #endif void clear() {head.next = &head; head.prev = &head;__TBB_store_relaxed(count, 0);} private: __TBB_atomic size_t count; node_t head; [/cpp]
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Yes, it is OK, the TBB_USE_DEBUG isn't needed here.
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Hi,
Could you give us more detail?
I don't seem to reproduce the issue.
temp.clear() is wrappend around by '#if TBB_USE_ASSERT', it is called when TBB_USE_ASSERT==1; and __TBB_ASSERT is defined when TBB_USE_ASSERT==1.
And, could you check if you have the latest TBB release?
thanks
Could you give us more detail?
I don't seem to reproduce the issue.
temp.clear() is wrappend around by '#if TBB_USE_ASSERT', it is called when TBB_USE_ASSERT==1; and __TBB_ASSERT is defined when TBB_USE_ASSERT==1.
And, could you check if you have the latest TBB release?
thanks
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
I suppose it is because someone patch the bug after I report it.
This is the code in version 20111130, which was the most recent one when I report the issue.
As you can see it uses TBB_USE_DEBUG instead of TBB_USE_ASSERT
[bash] } #if TBB_USE_DEBUG temp.clear(); #endif } [/bash]
This is the code in version 20111130, which was the most recent one when I report the issue.
As you can see it uses TBB_USE_DEBUG instead of TBB_USE_ASSERT
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
That seems to be the case. Thanks.!!
The fix will be in the next TBB release.
The fix will be in the next TBB release.
Reply
Topic Options
- Subscribe to RSS Feed
- Mark Topic as New
- Mark Topic as Read
- Float this Topic for Current User
- Bookmark
- Subscribe
- Printer Friendly Page