Umbraco-cms: [Umbraco 8.1.1] Error on save and publish for existing node after content creation in eventhandler

[Appstract-fjellvang]

With 8.1.0 and 8.1.0 We're having an issue with our event handlers.

Basically the error happens if we have an existing published node(Called A), with an attached Event handler that checks if some state changed on the node(A), creates a new node(Called B), and moves the saving node(A) to be a child of the new node (B) then NuCache will fail with a null Error.

This error only happens if node A was already published and node B did no exist. The error will not occur if you create a fresh node A.

Secondly it Doesn't error pre 8.1.0

Reproduction

1. Install a fresh umbraco 8.1.1 with the example site if preffered.
2. Create a new Document Type "Content Page Content" and allow "Content Page" as child, and vice versa
3. Subscribe a new event to Contentservice published like so:

        private void ContentService_Published(Umbraco.Core.Services.IContentService sender, Umbraco.Core.Events.ContentPublishedEventArgs e)
        {
            foreach (var item in e.PublishedEntities)
            {
                if (item.Name == "Page")
                {
                    var cs = Current.Services.ContentService;//.Move(item, 1149);
                    var newContent = cs.Create("Content", 1170, "contentPageContent");
                    cs.SaveAndPublish(newContent);
                    cs.Move(item, newContent.Id);
                }
            }
        }

1. Create a new Content Page under About us Called Page.
2. Verify that "Page" was moved beneath "Content" without error.

3. SaveAndPublish "Page" Again and see the error occuring:
   Exception System.NullReferenceException: Object reference not set to an instance of an object. at Umbraco.Web.PublishedCache.NuCache.ContentStore.ClearBranchLocked(ContentNode content) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 716 at Umbraco.Web.PublishedCache.NuCache.ContentStore.SetBranch(Int32 rootContentId, IEnumerable1 kits) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\ContentStore.cs:line 640
   at Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.NotifyLocked(IEnumerable1 payloads, Boolean& draftChanged, Boolean& publishedChanged) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedSnapshotService.cs:line 684 at Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.Notify(JsonPayload[] payloads, Boolean& draftChanged, Boolean& publishedChanged) in d:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedSnapshotService.cs:line 623 at Umbraco.Web.Cache.ContentCacheRefresher.Refresh(JsonPayload[] payloads) in d:\a\1\s\src\Umbraco.Web\Cache\ContentCacheRefresher.cs:line 100 at Umbraco.Core.Sync.ServerMessengerBase.DeliverLocal[TPayload](ICacheRefresher refresher, TPayload[] payload) in d:\a\1\s\src\Umbraco.Core\Sync\ServerMessengerBase.cs:line 165 at Umbraco.Core.Sync.ServerMessengerBase.Deliver[TPayload](ICacheRefresher refresher, TPayload[] payload) in d:\a\1\s\src\Umbraco.Core\Sync\ServerMessengerBase.cs:line 295 at Umbraco.Core.Sync.ServerMessengerBase.PerformRefresh[TPayload](ICacheRefresher refresher, TPayload[] payload) in d:\a\1\s\src\Umbraco.Core\Sync\ServerMessengerBase.cs:line 64 at Umbraco.Web.Cache.DistributedCache.RefreshByPayload[TPayload](Guid refresherGuid, IEnumerable1 payloads) in d:\a\1\s\src\Umbraco.Web\Cache\DistributedCache.cs:line 94
   at Umbraco.Web.Cache.DistributedCacheExtensions.RefreshContentCache(DistributedCache dc, TreeChange1[] changes) in d:\a\1\s\src\Umbraco.Web\Cache\DistributedCacheExtensions.cs:line 122 at Umbraco.Web.Cache.DistributedCacheBinder.ContentService_TreeChanged(IContentService sender, EventArgs args) in d:\a\1\s\src\Umbraco.Web\Cache\DistributedCacheBinder_Handlers.cs:line 196 at Umbraco.Core.Events.EventDefinition2.RaiseEvent() in d:\a\1\s\src\Umbraco.Core\Events\EventDefinition.cs:line 69
   at Umbraco.Core.Events.QueuingEventDispatcher.ScopeExitCompleted() in d:\a\1\s\src\Umbraco.Core\Events\QueuingEventDispatcher.cs:line 23
   at Umbraco.Core.Events.QueuingEventDispatcherBase.ScopeExit(Boolean completed) in d:\a\1\s\src\Umbraco.Core\Events\QueuingEventDispatcherBase.cs:line 337
   at Umbraco.Core.Scoping.Scope.<>c__DisplayClass72_0.b__1() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 436
   at Umbraco.Core.Scoping.Scope.TryFinally(Int32 index, Action[] actions) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 474
   at Umbraco.Core.Scoping.Scope.TryFinally(Int32 index, Action[] actions) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 478
   at Umbraco.Core.Scoping.Scope.RobustExit(Boolean completed, Boolean onException) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 422
   at Umbraco.Core.Scoping.Scope.DisposeLastScope() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 402
   at Umbraco.Core.Scoping.Scope.Dispose() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 365
   at Umbraco.Core.Services.Implement.ContentService.Move(IContent content, Int32 parentId, Int32 userId) in d:\a\1\s\src\Umbraco.Core\Services\Implement\ContentService.cs:line 1895
   at Umbraco8._1._1_event.LogWhenPublished.ContentService_Published(IContentService sender, ContentPublishedEventArgs e) in C:\projects\Umbraco8.1.1-event\Umbraco8.1.1-event\LogWhenPublished.cs:line 30
   at Umbraco.Core.Events.TypedEventHandler2.Invoke(TSender sender, TEventArgs e) at Umbraco.Core.Events.EventDefinition2.RaiseEvent() in d:\a\1\s\src\Umbraco.Core\Events\EventDefinition.cs:line 69
   at Umbraco.Core.Events.QueuingEventDispatcher.ScopeExitCompleted() in d:\a\1\s\src\Umbraco.Core\Events\QueuingEventDispatcher.cs:line 23
   at Umbraco.Core.Events.QueuingEventDispatcherBase.ScopeExit(Boolean completed) in d:\a\1\s\src\Umbraco.Core\Events\QueuingEventDispatcherBase.cs:line 337
   at Umbraco.Core.Scoping.Scope.<>c__DisplayClass72_0.b__1() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 436
   at Umbraco.Core.Scoping.Scope.TryFinally(Int32 index, Action[] actions) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 474
   at Umbraco.Core.Scoping.Scope.TryFinally(Int32 index, Action[] actions) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 478
   at Umbraco.Core.Scoping.Scope.RobustExit(Boolean completed, Boolean onException) in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 422
   at Umbraco.Core.Scoping.Scope.DisposeLastScope() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 402
   at Umbraco.Core.Scoping.Scope.Dispose() in d:\a\1\s\src\Umbraco.Core\Scoping\Scope.cs:line 365
   at Umbraco.Core.Services.Implement.ContentService.SaveAndPublish(IContent content, String culture, Int32 userId, Boolean raiseEvents) in d:\a\1\s\src\Umbraco.Core\Services\Implement\ContentService.cs:line 905
   at Umbraco.Web.Editors.ContentController.PublishInternal(ContentItemSave contentItem, String defaultCulture, String cultureForInvariantErrors, Boolean& wasCancelled, String[]& successfulCultures) in d:\a\1\s\src\Umbraco.Web\Editors\ContentController.cs:line 1217
   at Umbraco.Web.Editors.ContentController.PostSaveInternal(ContentItemSave contentItem, Func2 saveMethod, Func2 mapToDisplay) in d:\a\1\s\src\Umbraco.Web\Editors\ContentController.cs:line 728
   at Umbraco.Web.Editors.ContentController.PostSave(ContentItemSave contentItem) in d:\a\1\s\src\Umbraco.Web\Editors\ContentController.cs:line 599
   at lambda_method(Closure , Object , Object[] )
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.<>c__DisplayClass6_2.b__2(Object instance, Object[] methodParameters)
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ActionExecutor.Execute(Object instance, Object[] arguments)
   at System.Web.Http.Controllers.ReflectedHttpActionDescriptor.ExecuteAsync(HttpControllerContext controllerContext, IDictionary2 arguments, CancellationToken cancellationToken) --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ApiControllerActionInvoker.<InvokeActionAsyncCore>d__1.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Filters.ActionFilterAttribute.<CallOnActionExecutedAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.ActionFilterAttribute.<ExecuteActionFilterAsyncCore>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ActionFilterResult.<ExecuteAsync>d__5.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Filters.AuthorizationFilterAttribute.<ExecuteAuthorizationFilterAsyncCore>d__3.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Controllers.ExceptionFilterResult.<ExecuteAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Web.Http.Controllers.ExceptionFilterResult.<ExecuteAsync>d__6.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Dispatcher.HttpControllerDispatcher.<SendAsync>d__15.MoveNext()

4. if you now change the node name, so the event handler wont trigger, resave and publish and all should be good again.

Some images:

It seems like the error is being thrown in the recursive method "ClearBranchLocked" in the ContentStore.cs, where after a recurse call and return from clearBranchLocked it tries to get an ID from link.value.nextSiblingContentID, which has just been cleared.

---

_This item has been added to our backlog AB#2210_

[Appstract-fjellvang]

Update: Seems the error happens when the node you're moving via event handler has child elements attached to it.

[nul800sebastiaan]

That certainly seems strange @Appstract-fjellvang that you would not be able to move an item with it's children. I can reproduce the problem in 8.1.1, we'll need to have a look at it.

[enkelmedia]

I have the same issue when I try to publish a parent after a child has changed.

public void Initialize()
{
      ContentService.Published += ContentServiceOnPublished;
}

private void ContentServiceOnPublished(IContentService sender, ContentPublishedEventArgs e)
{
   foreach (var entity in e.PublishedEntities.Where(x=>x.ContentType.Alias == "child"))
   {
       var parent = _contentService.GetById(entity.ParentId);
       //do some stuff
       _contentService.SaveAndPublish(parent);
   }
}

[stevemegson]

ClearBranchLocked includes this loop through the node's children:

```c#
var id = content.FirstChildContentId;
while (id > 0)
{
var link = GetLinkedNode(id, "child");
ClearBranchLocked(link.Value);
id = link.Value.NextSiblingContentId;
}

Usually this works, but in some situations the recursive call to `ClearBranchLocked` will set `link.Value` to null rather than inserting a new link with a null value. Then we hit a null reference when we try to read `NextSiblingContentId`.

Instead, we can read `Value` once, so that we're not affected if it's changed later:

```c#
var id = content.FirstChildContentId;
while (id > 0)
{
    var child = GetLinkedNode(id, "child").Value;                
    ClearBranchLocked(child);
    id = child.NextSiblingContentId;
}

[zpqrtbnk]

Note: ClearBranchLocked ends up doing SetValueLocked which considers the linked list of values for the node. And then:

• If the list contains 1 or more value that belong to old generations (previous values for previous snapshots, which have not been collected yet) - add a null value on top of the list.
• If the list contains 1 unique value that belong to the current generation... it's never been public yet, really, so we just optimize and entirely remove the list.

In this latter case, yes, the ClearBranchLocked code may throw a nullref. Good catch! And the fix totally makes sense.

As for your original issue - I guess that since you are moving things around as part of one single transaction you end up in the precise situation where we clear a branch containing things that have just been added to it = we hit the optimized code path.

[nul800sebastiaan]

This has been confirmed to work by @jmayntzhusen so I'll close this, the fix will be available in 8.1.5 and if you need it now you can download the build for this fix here: https://umbraco.visualstudio.com/Umbraco%20Cms/_build/results?buildId=25102

It is the same build as 8.1.4 + this fix applied, so make sure you're on 8.1.4 first and use the dlls from that build.