[AMQNET-570] Error in Apache.NMS.ActiveMQ.Connection.Close()

• Type: Bug
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Affects Version/s: 1.7.2
• Component/s: ActiveMQ

Environment

Windows Server 2008 R2

• Assignee: Unassigned
• Reporter: john weidman
• Created: 01/Aug/17 20:04
• Updated: 02/Aug/17 20:22

Description

We have been having issues with our ActiveMQ server and we think we have tracked at least part of the issue to a bug in the Close function of the Apache.NMS.ActiveMQ.Connection class. In the close function it appear to try to cleanup temporary destinations here:

if(this.tempDests.Count > 0)
{
	// Make a copy of the destinations to delete, because the act of deleting
	// them will modify the collection.
	ActiveMQTempDestination[] tempDestsToDelete = new ActiveMQTempDestination[this.tempDests.Count];

	this.tempDests.Values.CopyTo(tempDestsToDelete, 0);
	foreach(ActiveMQTempDestination dest in tempDestsToDelete)
	{
		dest.Delete();
	}
}

We have seen the following exceptions:

• "System.NullReferenceException" on dest.Delete()
• "System.IndexOutOfRangeException: Index was outside the bounds of the array." on this.tempDests.Values.CopyTo(tempDestsToDelete, 0)
• "System.ArgumentException: Destination array is not long enough to copy all the items in the collection. Check array index and length." on this.tempDests.Values.CopyTo(tempDestsToDelete, 0)

I assume that this is because of the length of tempDests is changing during the close. It seems like a lock(this.tempDests.SyncRoot) would be necessary around this section, something like:

if(this.tempDests.Count > 0)
{
	lock (this.tempDests.SyncRoot)
	{
		// Make a copy of the destinations to delete, because the act of deleting
		// them will modify the collection.
		ActiveMQTempDestination[] tempDestsToDelete = new ActiveMQTempDestination[this.tempDests.Count];

		this.tempDests.Values.CopyTo(tempDestsToDelete, 0);
		foreach (ActiveMQTempDestination dest in tempDestsToDelete)
		{
			dest.Delete();
		}
	}
}

Because of the exceptions about 1 out of 1,000 connections in our system do not get closed, and causes issues with the performance of ActiveMQ, to the point of needing restarting every few days.