You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@continuum.apache.org by ct...@apache.org on 2009/08/10 09:25:33 UTC

svn commit: r802673 - /continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java

Author: ctan
Date: Mon Aug 10 07:25:33 2009
New Revision: 802673

URL: http://svn.apache.org/viewvc?rev=802673&view=rev
Log:
[CONTINUUM-2320] prevent NPE in build agent task queue manager

Modified:
    continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java

Modified: continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java?rev=802673&r1=802672&r2=802673&view=diff
==============================================================================
--- continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java (original)
+++ continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java Mon Aug 10 07:25:33 2009
@@ -116,8 +116,11 @@
             {
                 for ( BuildProjectTask task : queues )
                 {
-                    log.info( "remove project '" + task.getProjectName() + "' from build queue" );
-                    buildAgentBuildQueue.remove( task );
+                    if ( task != null )
+                    {
+                        log.info( "remove project '" + task.getProjectName() + "' from build queue" );
+                        buildAgentBuildQueue.remove( task );
+                    }
                 }
             }
             else
@@ -185,7 +188,7 @@
             {
                 for ( BuildProjectTask task : queues )
                 {
-                    if ( task.getProjectId() == projectId )
+                    if ( task != null && task.getProjectId() == projectId )
                     {
                         log.info( "project already in build queue" );
                         return true;
@@ -216,8 +219,8 @@
             {
                 for ( PrepareBuildProjectsTask task : queues )
                 {
-                    if ( task.getProjectGroupId() == projectGroupId && task.getTrigger() == trigger &&
-                        task.getScmRootAddress().equals( scmRootAddress ) )
+                    if ( task != null && task.getProjectGroupId() == projectGroupId && 
+                         task.getTrigger() == trigger && task.getScmRootAddress().equals( scmRootAddress ) )
                     {
                         log.info( "projects already in build queue" );
                         return true;
@@ -299,7 +302,7 @@
         {
             for ( PrepareBuildProjectsTask task : tasks )
             {
-                if ( task.getProjectGroupId() == projectGroupId && task.getScmRootId() == scmRootId )
+                if ( task != null && task.getProjectGroupId() == projectGroupId && task.getScmRootId() == scmRootId )
                 {
                     return getPrepareBuildQueue().remove( task );
                 }
@@ -318,7 +321,7 @@
         {
             for ( PrepareBuildProjectsTask task : tasks )
             {
-                if ( ArrayUtils.contains( hashCodes, task.getHashCode() ) )
+                if ( task != null && ArrayUtils.contains( hashCodes, task.getHashCode() ) )
                 {
                     getPrepareBuildQueue().remove( task );
                 }
@@ -335,7 +338,7 @@
         {
             for ( BuildProjectTask task : tasks )
             {
-                if ( task.getProjectId() == projectId && task.getBuildDefinitionId() == buildDefinitionId )
+                if ( task != null && task.getProjectId() == projectId && task.getBuildDefinitionId() == buildDefinitionId )
                 {
                     return getBuildQueue().remove( task );
                 }
@@ -354,7 +357,7 @@
         {
             for ( BuildProjectTask task : tasks )
             {
-                if ( ArrayUtils.contains( hashCodes, task.getHashCode() ) )
+                if ( task != null && ArrayUtils.contains( hashCodes, task.getHashCode() ) )
                 {
                     getBuildQueue().remove( task );
                 }



Re: svn commit: r802673

Posted by Marica Tan <ma...@gmail.com>.
Hi Brett,

I added the check above just in case the build queue has a null task. But I
reviewed the code and it seems it's not possible to insert a null task in
the queue.

I can remove the checking for null if you want.


org.apache.continuum.buildagent.action.CreateBuildProjectTaskAction @ line
66


Thanks,
--
Marica


On Mon, Sep 14, 2009 at 4:06 PM, Brett Porter <br...@apache.org> wrote:

> Ping? :)
>
>
> On 28/08/2009, at 5:14 PM, Brett Porter wrote:
>
>  Marica?
>>
>> On 22/08/2009, at 10:28 PM, Brett Porter wrote:
>>
>>
>>> On 10/08/2009, at 3:25 AM, ctan@apache.org wrote:
>>>
>>>  Author: ctan
>>>> Date: Mon Aug 10 07:25:33 2009
>>>> New Revision: 802673
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=802673&view=rev
>>>> Log:
>>>> [CONTINUUM-2320] prevent NPE in build agent task queue manager
>>>>
>>>> Modified:
>>>>
>>>>  continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>>>
>>>> Modified:
>>>> continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java?rev=802673&r1=802672&r2=802673&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>>> (original)
>>>> +++
>>>> continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>>> Mon Aug 10 07:25:33 2009
>>>> @@ -116,8 +116,11 @@
>>>>          {
>>>>              for ( BuildProjectTask task : queues )
>>>>              {
>>>> -                    log.info( "remove project '" +
>>>> task.getProjectName() + "' from build queue" );
>>>> -                    buildAgentBuildQueue.remove( task );
>>>> +                    if ( task != null )
>>>> +                    {
>>>> +                        log.info( "remove project '" +
>>>> task.getProjectName() + "' from build queue" );
>>>> +                        buildAgentBuildQueue.remove( task );
>>>> +                    }
>>>>
>>>
>>>
>>> Would it be better to prevent the insertion of a null task into the queue
>>> instead? This seems like it might hide an error on the other end.
>>>
>>> - Brett
>>>
>>>
>>
>

Re: svn commit: r802673

Posted by Brett Porter <br...@apache.org>.
Ping? :)

On 28/08/2009, at 5:14 PM, Brett Porter wrote:

> Marica?
>
> On 22/08/2009, at 10:28 PM, Brett Porter wrote:
>
>>
>> On 10/08/2009, at 3:25 AM, ctan@apache.org wrote:
>>
>>> Author: ctan
>>> Date: Mon Aug 10 07:25:33 2009
>>> New Revision: 802673
>>>
>>> URL: http://svn.apache.org/viewvc?rev=802673&view=rev
>>> Log:
>>> [CONTINUUM-2320] prevent NPE in build agent task queue manager
>>>
>>> Modified:
>>>  continuum/branches/continuum-1.3.x/continuum-buildagent/continuum- 
>>> buildagent-core/src/main/java/org/apache/continuum/buildagent/ 
>>> taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>>
>>> Modified: continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>>> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>> URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java?rev=802673&r1=802672&r2=802673&view=diff
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>>> buildagent/taskqueue/manager/ 
>>> DefaultBuildAgentTaskQueueManager.java (original)
>>> +++ continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>>> buildagent/taskqueue/manager/ 
>>> DefaultBuildAgentTaskQueueManager.java Mon Aug 10 07:25:33 2009
>>> @@ -116,8 +116,11 @@
>>>           {
>>>               for ( BuildProjectTask task : queues )
>>>               {
>>> -                    log.info( "remove project '" +  
>>> task.getProjectName() + "' from build queue" );
>>> -                    buildAgentBuildQueue.remove( task );
>>> +                    if ( task != null )
>>> +                    {
>>> +                        log.info( "remove project '" +  
>>> task.getProjectName() + "' from build queue" );
>>> +                        buildAgentBuildQueue.remove( task );
>>> +                    }
>>
>>
>> Would it be better to prevent the insertion of a null task into the  
>> queue instead? This seems like it might hide an error on the other  
>> end.
>>
>> - Brett
>>
>


Re: svn commit: r802673

Posted by Brett Porter <br...@apache.org>.
Marica?

On 22/08/2009, at 10:28 PM, Brett Porter wrote:

>
> On 10/08/2009, at 3:25 AM, ctan@apache.org wrote:
>
>> Author: ctan
>> Date: Mon Aug 10 07:25:33 2009
>> New Revision: 802673
>>
>> URL: http://svn.apache.org/viewvc?rev=802673&view=rev
>> Log:
>> [CONTINUUM-2320] prevent NPE in build agent task queue manager
>>
>> Modified:
>>   continuum/branches/continuum-1.3.x/continuum-buildagent/continuum- 
>> buildagent-core/src/main/java/org/apache/continuum/buildagent/ 
>> taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>>
>> Modified: continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>> URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java?rev=802673&r1=802672&r2=802673&view=diff
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java  
>> (original)
>> +++ continuum/branches/continuum-1.3.x/continuum-buildagent/ 
>> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
>> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java  
>> Mon Aug 10 07:25:33 2009
>> @@ -116,8 +116,11 @@
>>            {
>>                for ( BuildProjectTask task : queues )
>>                {
>> -                    log.info( "remove project '" +  
>> task.getProjectName() + "' from build queue" );
>> -                    buildAgentBuildQueue.remove( task );
>> +                    if ( task != null )
>> +                    {
>> +                        log.info( "remove project '" +  
>> task.getProjectName() + "' from build queue" );
>> +                        buildAgentBuildQueue.remove( task );
>> +                    }
>
>
> Would it be better to prevent the insertion of a null task into the  
> queue instead? This seems like it might hide an error on the other  
> end.
>
> - Brett
>


Re: svn commit: r802673

Posted by Brett Porter <br...@apache.org>.
On 10/08/2009, at 3:25 AM, ctan@apache.org wrote:

> Author: ctan
> Date: Mon Aug 10 07:25:33 2009
> New Revision: 802673
>
> URL: http://svn.apache.org/viewvc?rev=802673&view=rev
> Log:
> [CONTINUUM-2320] prevent NPE in build agent task queue manager
>
> Modified:
>    continuum/branches/continuum-1.3.x/continuum-buildagent/continuum- 
> buildagent-core/src/main/java/org/apache/continuum/buildagent/ 
> taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
>
> Modified: continuum/branches/continuum-1.3.x/continuum-buildagent/ 
> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java
> URL: http://svn.apache.org/viewvc/continuum/branches/continuum-1.3.x/continuum-buildagent/continuum-buildagent-core/src/main/java/org/apache/continuum/buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java?rev=802673&r1=802672&r2=802673&view=diff
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- continuum/branches/continuum-1.3.x/continuum-buildagent/ 
> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java  
> (original)
> +++ continuum/branches/continuum-1.3.x/continuum-buildagent/ 
> continuum-buildagent-core/src/main/java/org/apache/continuum/ 
> buildagent/taskqueue/manager/DefaultBuildAgentTaskQueueManager.java  
> Mon Aug 10 07:25:33 2009
> @@ -116,8 +116,11 @@
>             {
>                 for ( BuildProjectTask task : queues )
>                 {
> -                    log.info( "remove project '" +  
> task.getProjectName() + "' from build queue" );
> -                    buildAgentBuildQueue.remove( task );
> +                    if ( task != null )
> +                    {
> +                        log.info( "remove project '" +  
> task.getProjectName() + "' from build queue" );
> +                        buildAgentBuildQueue.remove( task );
> +                    }


Would it be better to prevent the insertion of a null task into the  
queue instead? This seems like it might hide an error on the other end.

- Brett