You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@continuum.apache.org by Brett Porter <br...@apache.org> on 2009/08/22 14:28:27 UTC
Re: svn commit: r802673
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 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
>