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 01:00:47 UTC

[jira] Created: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

ThreadManagerTests.h blockTest can be more thorough
---------------------------------------------------

                 Key: THRIFT-466
                 URL: https://issues.apache.org/jira/browse/THRIFT-466
             Project: Thrift
          Issue Type: Bug
          Components: Test Suite
    Affects Versions: 0.1
         Environment: Mac OS X 10.5.6
            Reporter: Rush Manbert


At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.

There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.

I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706259#action_12706259 ] 

Rush Manbert commented on THRIFT-466:
-------------------------------------

Yeah, that would make sense. The unpatched test has an unfortunate tendency to hang as well and the patch doesn't change any of that.

I guess there are 2 ways to approach this one. You could run it in a debugger and step through the new code and see that it does the right thing. That's what I did.

Or you could apply both patches from THRIFT-487, see that concurrency_test (mostly) runs, then apply this patch and see that nothing breaks. But you probably still need to step through it to convince yourself that it's right.

I hate dealing with software sometimes...

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

David Reiss resolved THRIFT-466.
--------------------------------

       Resolution: Fixed
    Fix Version/s: 0.2

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             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: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "Marc Kwiatkowski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706333#action_12706333 ] 

Marc Kwiatkowski commented on THRIFT-466:
-----------------------------------------

This looks good.  Thanks

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "Marc Kwiatkowski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706334#action_12706334 ] 

Marc Kwiatkowski commented on THRIFT-466:
-----------------------------------------

Nice catch.  This looks good.

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706217#action_12706217 ] 

David Reiss commented on THRIFT-466:
------------------------------------

The test hangs for me when I run with this patch.  Can anyone else reproduce?

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "Marc Kwiatkowski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Marc Kwiatkowski updated THRIFT-466:
------------------------------------

    Comment: was deleted

(was: Nice catch.  This looks good.)

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-466) ThreadManagerTests.h blockTest can be more thorough

Posted by "Rush Manbert (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-466?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rush Manbert updated THRIFT-466:
--------------------------------

    Attachment: ThreadManagerTestsPatch.txt

Patch file.

> ThreadManagerTests.h blockTest can be more thorough
> ---------------------------------------------------
>
>                 Key: THRIFT-466
>                 URL: https://issues.apache.org/jira/browse/THRIFT-466
>             Project: Thrift
>          Issue Type: Bug
>          Components: Test Suite
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>         Attachments: ThreadManagerTestsPatch.txt
>
>
> At the point where the blockTest has created 100 tasks and it tries to add one more, but expects to timeout it does not catch TooManyPendingTasksException, whici is thrown if there is a bug in the call chain represented by canSleep(). In my case, my Boost thread implementation had a bug in the getCurrentThreadId() code that caused canSleep() to return false, which caused the exception to be thrown.
> There is also no test for attempting to add the extra thread with a negative timeout value. In this case, TooManyPendingTasksException should be thrown and anything else is a bug.
> I have a patch that modifies the existing try block and adds a second one that attempts to add the extra thread with a negative timeout value.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.