none
Do I Need to Call Dispose() on This?

    Question

  • Do I need to call Dispose() on the item.ParentList.ParentWeb in this code? Since I am passing the SPListItem ByRef, I wasn't sure how that would affect things. 



    1Public Sub SetPermissionsApprove(ByRef item As SPListItem, ByVal principal As SPPrincipal) 
    2        Dim roleDefinitions As SPRoleDefinitionCollection = item.ParentList.ParentWeb.RoleDefinitions 
    3        Dim roleAssignments As SPRoleAssignmentCollection = item.RoleAssignments 
    4 
    5        Dim roleAssignment As New SPRoleAssignment(principal) 
    6        Dim roleDefBindings As SPRoleDefinitionBindingCollection 
    7 
    8        roleDefBindings = roleAssignment.RoleDefinitionBindings 
    9 
    10        roleDefBindings.Add(roleDefinitions.Item("Approve Tasks")) 
    11        roleAssignments.Add(roleAssignment) 
    12    End Sub 


    --Josh Winfree
    Thursday, January 22, 2009 7:24 PM

Answers

  • In your case, I would actually just reference item.Web, which defers to  item.ListItems.List.Lists.Web which does not have a chance to open a new SPWeb reference.

    If you do need to use SPList.ParentWeb, take a look at my analysis here. In most cases, ParentWeb will not open a new SPWeb reference. You can test this by comparing list.ParentWeb with list.Lists.Web (which will only ever return a reference to the SPWeb from which it was created): only if they are different was a new SPWeb created that needs to be disposed.

    If you're not sure, you should always err on the side of not disposing. It's usually easier to observe memory pressure (and use Stefan's troubleshooting techniques) than it is to track down issues related to the use of an SPWeb that was disposed too early.

    Regards ~

    Keith

    • Proposed as answer by Keith DahlbyMVP Friday, January 23, 2009 2:24 PM
    • Marked as answer by J. Winfree Friday, January 23, 2009 3:30 PM
    Friday, January 23, 2009 2:17 PM

All replies

  • Yes I believe you would. I would try to take the reference to the SPWeb out of the one line of code like that though to make it more obvious that it is there as well (thats just me though I guess) but you will need to dispose of it.
    Brian Farnhill
    Microsoft Certified Application Developer
    http://pointstoshare.spaces.live.com
    Canberra SharePoint User Group
    Thursday, January 22, 2009 7:35 PM
  • I have to use the SPWeb (ParentWeb) in order to get the RoleDefinitions. This code is actually running in a workflow but shouldn't change things. Do you know of a better or cleaner way of adding custom permissions for someone?
    --Josh Winfree
    Thursday, January 22, 2009 7:41 PM
  • Here are the 4 best references I know of about properly disposing of SPWeb and SPSite objects (they also mention a few other objects that need to be cleaned up)

    In addition MS is coming out with a tool that is basically like FxCop that will check for SPWeb/SPSite objects, but its not out yet.

    The tricky thing that most of the articles don't really discuss that much, is when you use statements like you have above
    "item.ParentList.ParentWeb.RoleDefinitions" that technically creates an SPWeb object in your method when you call the ParentWeb.  You therefore should probably break that line out to something like 

    using(SPWeb parent = item.ParentList.ParentWeb) 
       //your code here 

    Tony Testa www.tonytestasworld.com
    Friday, January 23, 2009 3:32 AM
  • In your case, I would actually just reference item.Web, which defers to  item.ListItems.List.Lists.Web which does not have a chance to open a new SPWeb reference.

    If you do need to use SPList.ParentWeb, take a look at my analysis here. In most cases, ParentWeb will not open a new SPWeb reference. You can test this by comparing list.ParentWeb with list.Lists.Web (which will only ever return a reference to the SPWeb from which it was created): only if they are different was a new SPWeb created that needs to be disposed.

    If you're not sure, you should always err on the side of not disposing. It's usually easier to observe memory pressure (and use Stefan's troubleshooting techniques) than it is to track down issues related to the use of an SPWeb that was disposed too early.

    Regards ~

    Keith

    • Proposed as answer by Keith DahlbyMVP Friday, January 23, 2009 2:24 PM
    • Marked as answer by J. Winfree Friday, January 23, 2009 3:30 PM
    Friday, January 23, 2009 2:17 PM
  • Tony Testa said:

    The tricky thing that most of the articles don't really discuss that much, is when you use statements like you have above
    "item.ParentList.ParentWeb.RoleDefinitions" that technically creates an SPWeb object in your method when you call the ParentWeb.  You therefore should probably break that line out to something like 

    using(SPWeb parent = item.ParentList.ParentWeb) 
       //your code here 

    Tony Testa www.tonytestasworld.com



    Tony,

    I agree about using the Using statement, only thing is, this workflow, and most of my custom code is in VB and not C#. Just personal preference really.

    --Josh Winfree
    Friday, January 23, 2009 3:26 PM
  • Keith,

    I must have somehow overlooked the fact that I could do item.Web instead of how I was doing it. Thanks for your help.

    --Josh Winfree
    Friday, January 23, 2009 3:30 PM
  • Josh,

    My bad ... but just as an FYI VB has a using statement as well actually

    Using A as new SomethingThatImplementsIDisposable
      'Do something with A
    End Using

    Tony Testa www.tonytestasworld.com
    Friday, January 23, 2009 3:33 PM
  • Josh Winfree said:
    I agree about using the Using statement, only thing is, this workflow, and most of my custom code is in VB and not C#. Just personal preference really.

    --Josh Winfree

    I don't believe you need 'using' in this case, but VB has an equivalent syntax:

    Using web As item.ParentList.ParentWeb 
      ' Your code here
    End Using
    Friday, January 23, 2009 3:34 PM
  • Tony,

    Maybe I was just somehow ignorant to the fact that VB had a Using statement. I thought I had heard in the past that it didn't. Guess I should have done some more research for myself.

    Learn something new everyday ya know?..

    Thanks!

    --Josh Winfree
    Friday, January 23, 2009 4:03 PM