You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Rush Manbert (JIRA)" <ji...@apache.org> on 2009/04/23 00:50:47 UTC
[jira] Created: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
ThreadFactoryTests.h increments iterator after erasing the element to which it refers
-------------------------------------------------------------------------------------
Key: THRIFT-465
URL: https://issues.apache.org/jira/browse/THRIFT-465
Project: Thrift
Issue Type: Bug
Components: Test Suite
Affects Versions: 0.1
Environment: Mac OS X 10.5.6
Reporter: Rush Manbert
Attachments: ThreadFactoryTestsPatch.txt
In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
threads.erase(*thread);
}
but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
I believe the equivalent is this:
while (!threads.empty()) {
threads.erase (*(threads.begin()));
}
although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716106#action_12716106 ]
David Reiss commented on THRIFT-465:
------------------------------------
Can we just delete this entire block since the set's destructor will delete all of the elements?
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rush Manbert updated THRIFT-465:
--------------------------------
Attachment: ThreadFactoryTestsPatch.txt
Patch file.
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706527#action_12706527 ]
Rush Manbert commented on THRIFT-465:
-------------------------------------
Should I submit a different patch that calls clear?
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "Marc Kwiatkowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706331#action_12706331 ]
Marc Kwiatkowski commented on THRIFT-465:
-----------------------------------------
I think clear is ok here too.
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-465.
--------------------------------
Resolution: Fixed
Fix Version/s: 0.2
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Fix For: 0.2
>
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-465) ThreadFactoryTests.h increments
iterator after erasing the element to which it refers
Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-465?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716115#action_12716115 ]
Rush Manbert commented on THRIFT-465:
-------------------------------------
That works too. But I see that you resolved it already. What did you end up doing?
> ThreadFactoryTests.h increments iterator after erasing the element to which it refers
> -------------------------------------------------------------------------------------
>
> Key: THRIFT-465
> URL: https://issues.apache.org/jira/browse/THRIFT-465
> Project: Thrift
> Issue Type: Bug
> Components: Test Suite
> Affects Versions: 0.1
> Environment: Mac OS X 10.5.6
> Reporter: Rush Manbert
> Fix For: 0.2
>
> Attachments: ThreadFactoryTestsPatch.txt
>
>
> In lib/cpp/src/consurrency/test/ThreadFactoryTests.h the reapNThreads() function has this loop:
> for (std::set<shared_ptr<Thread> >::const_iterator thread = threads.begin(); thread != threads.end(); thread++) {
> threads.erase(*thread);
> }
> but erasing the element from the container invalidates all iteratots that point to that element, so this may result in undefined behavior.
> I believe the equivalent is this:
> while (!threads.empty()) {
> threads.erase (*(threads.begin()));
> }
> although it really seems like you could just call threads.clear() as well and forgo the loop. But maybe there's some subtlety here that I'm missing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.