You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Kristian Waagan (Created) (JIRA)" <ji...@apache.org> on 2011/10/04 14:29:33 UTC

[jira] [Created] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

SpawnedProcess.complete may fail to destroy the process when a timeout is specified
-----------------------------------------------------------------------------------

                 Key: DERBY-5444
                 URL: https://issues.apache.org/jira/browse/DERBY-5444
             Project: Derby
          Issue Type: Bug
          Components: Test
    Affects Versions: 10.9.0.0
            Reporter: Kristian Waagan
            Assignee: Kristian Waagan


The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Closed] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Closed) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan closed DERBY-5444.
----------------------------------


Closing issue.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>             Fix For: 10.8.2.2, 10.9.0.0
>
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff, derby-5444-1c-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Dag H. Wanvik (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13121109#comment-13121109 ] 

Dag H. Wanvik commented on DERBY-5444:
--------------------------------------

Changes look good, just wondering, what is the test use case for the way interrupts are handled here? Is this code liable to be used in a context where
the current thread is being interrupted? (since you turn them back on consistently - not that it should hurt)
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan resolved DERBY-5444.
------------------------------------

          Resolution: Fixed
       Fix Version/s: 10.9.0.0
    Issue & fix info:   (was: Patch Available)

Thanks for the reviews, Dag.

Resolving issue.
I don't expect to do more work here.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>             Fix For: 10.9.0.0
>
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff, derby-5444-1c-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-5444:
-----------------------------------

    Attachment: derby-5444-1b-destroy_on_timeout.diff

Patch 1b changes how InterruptedExceptions are handled; they are now either ignored (because it doesn't matter for SpawnedProcess), or an exception is raised.

Running regressions now.
Patch ready for review.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13121419#comment-13121419 ] 

Kristian Waagan commented on DERBY-5444:
----------------------------------------

I don't know of any use cases for the interrupt handling introduced by the patch currently. Restoring the interrupted status seemed more reasonable than swallowing them, as it gives the caller  a chance to detect that the thread has been interrupted (i.e. Thread.interrupted or Thread.isInterrupted).

The patch raises an exception if the thread is interrupted while waiting for the spawned process to die. I'll do another pass and improve the error handling.
Off the top of my head, maybe the following:
 o method sleep: ignore interrupt, allow premature wakeup, leave interrupted status cleared
 o javaProcess.waitFor: throw exception, leave interrupted status cleared
 o xSaver.join: not sure about this one. Would it be best to retry, or simply continue (as with current patch)?

As you say, I don't think the test framework will interrupt the thread in the normal case. The reason for not restoring the interrupted flag in the cases were we don't handle it must be that it can cause other code to fall over, i.e. IO calls in another unrelated test. The caller probably can't do anything to recover from the interrupts, so it would maybe be better to swallow them after all (except for in the javaProcess.waitFor case).
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Dag H. Wanvik (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13121622#comment-13121622 ] 

Dag H. Wanvik commented on DERBY-5444:
--------------------------------------

Thanks, Kristian. I think, if we don't expect any interrupts to happen that it's good to just fail the test, since probably it would be assign that something else is wrong - better to fail early. I see now you ignore it in two cases: sleep (where it doesn't really matter as you say) and while waiting for the two streamsavers to terminate. You throw in the case where we get interrupted before we are able to see the spawned process terminate and get its error code. I guess that protects the main functionality, but I think I'd prefer throwing in the other two cases as well (from the fail early principle). Unless you can see a reason it would be good to continue here? But that is just my preference, your call.
+1 in any case.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-5444:
-----------------------------------

    Attachment: derby-5444-1a-destroy_on_timeout.diff

Attaching patch 1a, which includes the following changes:
 o don't throw InterruptedException when sleeping.
    This cluttered the calling code, and instead I restored the interrupted status of the thread and continued work.
 o rewrote loop/login in complete to make sure we always destroy the process if a timeout is specified and we indeed time out.
 o print a warning if we are interrupted while waiting for the output from the process to be gathered.

Regression tests passed on Solaris11.
Patch ready for review.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-5444:
-----------------------------------

    Attachment: derby-5444-1c-destroy_on_timeout.diff

Attaching patch 1c, which only fixes the loop logic.

I went a bit back and forth on the handling of InterruptedException, and decided to change nothing. SpawnedProcess is mostly used in setUp and tearDown methods anyway, and they are usually defined to throw Exception.

As far as I understand, only Object.wait is subject to spurious wakeups, so in the other cases another thread must have explicitly interrupted the sleeping/working thread.

Committed 1c to trunk with revision 1179546.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff, derby-5444-1c-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Myrna van Lunteren (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13121936#comment-13121936 ] 

Myrna van Lunteren commented on DERBY-5444:
-------------------------------------------

Would this be suitable for back port to the 10.8 branch?
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>             Fix For: 10.9.0.0
>
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff, derby-5444-1c-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-5444:
-----------------------------------

    Fix Version/s: 10.8.2.2

Myrna, I backported the fix to the 10.8 branch with revision 1180817.
                
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>             Fix For: 10.8.2.2, 10.9.0.0
>
>         Attachments: derby-5444-1a-destroy_on_timeout.diff, derby-5444-1b-destroy_on_timeout.diff, derby-5444-1c-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5444) SpawnedProcess.complete may fail to destroy the process when a timeout is specified

Posted by "Kristian Waagan (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5444?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kristian Waagan updated DERBY-5444:
-----------------------------------

    Issue & fix info: Patch Available
    
> SpawnedProcess.complete may fail to destroy the process when a timeout is specified
> -----------------------------------------------------------------------------------
>
>                 Key: DERBY-5444
>                 URL: https://issues.apache.org/jira/browse/DERBY-5444
>             Project: Derby
>          Issue Type: Bug
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Kristian Waagan
>            Assignee: Kristian Waagan
>         Attachments: derby-5444-1a-destroy_on_timeout.diff
>
>
> The logic in SpawnedProcess has a weakness that may result in the wrapped process not being destroyed if the destroy variable is false and a timeout is specified.
> The problem is that the while condition will shortcut the if condition in the catch clause (where destroy is set to true if the timeout is exceeded).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira