I’ve been having a problem with an app I wrote. It works great, 99% of the time, but sometimes the ItemAdded event appears not to fire.
Now this is using a list with over 20,000 entries in it and its growing all the time. An associated list that references this list as a lookup has over 144,000 entries in it.
The event firing problem is becoming more frequent now as more users start using the system, there are at least 20 occurances in that 20,000 items, so it has to be fixed.
Managed to track down an error in the log
System.Data.SqlClient.SqlException: Transaction (Process ID 61) was deadlocked on lock | communication buffer resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
It’s stack trace was similar too this (I cut it down a bit and changed some names, my naming is not this bad)
at Microsoft.SharePoint.SPListItemCollection.EnsureListItemsData()
at Microsoft.SharePoint.SPListItemCollection.get_Item(Guid uniqueId)
at BinaryJam.Core.Code.TicketListReceiver.ItemReceiver.ItemAddedActual(Guid SiteId, Guid ListItemId, String UserDisplayName)
at BinaryJam.Core.Code.TicketListReceiver.ItemReceiver.<>c__DisplayClass11.<ItemAdded>b__e()
at Microsoft.SharePoint.SPSecurity.CodeToRunElevatedWrapper(Object state)
at Microsoft.SharePoint.SPSecurity.<>c__DisplayClass4.<RunWithElevatedPrivileges>b__2()
at Microsoft.SharePoint.Utilities.SecurityContext.RunAsProcess(CodeToRunElevated secureCode)
at Microsoft.SharePoint.SPSecurity.RunWithElevatedPrivileges(WaitCallback secureCode, Object param)
at Microsoft.SharePoint.SPSecurity.RunWithElevatedPrivileges(CodeToRunElevated secureCode)
at BinaryJam.Core.Code.ItemListReceiver.ItemReceiver.ItemAdded(SPItemEventProperties properties)
So as you can see, it explodes on the GetItem.
This was my stupid mistake, a mistake I bet many of you are making every day.
SPListItem item = List.Items[listItemId];
This I believe is the primary cause of the problem as this statement loads the complete List before returning the specific item I needed and as this list grows, remember 20,000 items already, its going to get worse. Now for most of you this won’t be a problem as you will be just getting the ListItem from the parameters, in my case I have to run as elevated as Im doing lots of stuff with Security Groups, that the entering user may not be in, hence my needing to re-get the List Item in the right security context.
So if your having a problem like this remember, Always use the
List.GetItemByID(id); //Or
List.GetItemByUniqueId(guid);
This may or may not solve my problem I believe it will, if it doesn’t it’s still going to improve the performance of my application. If your wondering, this is the only time I use this and don’t know why I did, the rest of the App uses getby id or SPQueries.
A Good Article on Performance I did actually read, hence the app performs pretty well, except for this one bug, which is now hopefully fixed.
February 19, 2010 at 12:05 pm
Nice post. Just wanted to point out a bigger problem in this line of code:
SPListItem item = List.Items[listItemId]
As you are aware you are getting the item by index. You are assuming here that the List Item Id will match the index. I suppose the value your listItemId actually stores is (The Real List Item Id -1)? In any case passing the list item id in this scenario could have potentially very bad results.
Lets say your list has 4 items. Someone comes along and deletes the 3rd item with id 3. Now your code tries to get item with id 4:
SPListItem item = List.Items[listItemId]; (where listitemid = actual list item id -1 i.e. 3)
You will get a index out of range error because there are only 3 items in your collection now.
Now someone comes along and adds another list item. Your code again tries to get item with id 4:
SPListItem item = List.Items[listItemId]; (where listitemid = real list item id – 1 i.e. 3)
This time there wont be a crash because there are 4 items in the collection. But it wont get the item with id 4 instead it will pick up the item with id 5 because in this scenario it is 3rd in the collection index. In other words you end up making changes to a totally different list item.
So your fix actually fixed 2 of your problems. I can only assume no one ever deleted a list item from your list.
February 22, 2010 at 9:41 am
Actually in my code, listItemId was just poorly named, I was not referencing anything by it’s index, listItemId should have been called listItemUniqueId.
Had I been using an index then the comment is correct, accessing things by index then someone comes along and removes it in the millisecond between the event receiver firing and passing me the ID and my retrieval of it in higher priviledges could lead to getting the wrong item. So well spotted.
Luckily I’m not so stupid as to use an index, but I have been sloppy in my naming! 😉
February 22, 2010 at 2:29 pm
Cheers! Sorry for assuming you meant list item id although I must admit I did think that something else was going on in the background. If you were really using the list item id you would have used something like list.items[listitemid – 1]. So my apologies there.
I recently came across a piece of code that was doing exactly the thing I was describing and it caused all sorts of chaos as you can imagine 🙂