The moveDoneContext must must live longer than the PendingLidTracker::Token.

Since you either have a FeedToken or a moveDoneContext, and as they share the same IDestructorCallback as base class,
it is enough with one token. This token is destructed after the PendingLidTracker::Token.

10 files changed, 31 insertions, 98 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.cpp b/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.cpp
index 31e4c87b352..445f21044a8 100644
--- a/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.cpp
@@ -1,24 +1,14 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "operationdonecontext.h"
-#include <vespa/searchcore/proton/common/feedtoken.h>
 
 namespace proton {
 
-OperationDoneContext::OperationDoneContext(FeedToken token)
+OperationDoneContext::OperationDoneContext(IDestructorCallback::SP token)
     : _token(std::move(token))
 {
 }
 
-OperationDoneContext::~OperationDoneContext()
-{
-    ack();
-}
-
-void
-OperationDoneContext::ack()
-{
-    _token.reset();
-}
+OperationDoneContext::~OperationDoneContext() = default;
 
 }  // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.h b/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.h
index 16b6a320b66..ccd7721fe25 100644
--- a/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.h
+++ b/searchcore/src/vespa/searchcore/proton/server/operationdonecontext.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include <vespa/vespalib/util/idestructorcallback.h>
-#include <vespa/searchcore/proton/common/feedtoken.h>
 
 namespace proton {
 
@@ -16,13 +15,14 @@ namespace proton {
  */
 class OperationDoneContext : public vespalib::IDestructorCallback
 {
-    FeedToken _token;
-    void ack();
 public:
-    OperationDoneContext(FeedToken token);
+    using IDestructorCallback = vespalib::IDestructorCallback;
+    OperationDoneContext(IDestructorCallback::SP token);
 
     ~OperationDoneContext() override;
     bool hasToken() const { return static_cast<bool>(_token); }
+private:
+    IDestructorCallback::SP _token;
 };
 
 }  // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/putdonecontext.cpp b/searchcore/src/vespa/searchcore/proton/server/putdonecontext.cpp
index 23caaf1250b..ac4f79ca89b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/putdonecontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/putdonecontext.cpp
@@ -9,9 +9,8 @@ using document::Document;
 
 namespace proton {
 
-PutDoneContext::PutDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted,
-                               std::shared_ptr<const Document> doc,
-                               uint32_t lid)
+PutDoneContext::PutDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted,
+                               std::shared_ptr<const Document> doc, uint32_t lid)
     : OperationDoneContext(std::move(token)),
       _uncommitted(std::move(uncommitted)),
       _lid(lid),
diff --git a/searchcore/src/vespa/searchcore/proton/server/putdonecontext.h b/searchcore/src/vespa/searchcore/proton/server/putdonecontext.h
index e7271d8a1b3..66bae97ad02 100644
--- a/searchcore/src/vespa/searchcore/proton/server/putdonecontext.h
+++ b/searchcore/src/vespa/searchcore/proton/server/putdonecontext.h
@@ -28,9 +28,8 @@ class PutDoneContext : public OperationDoneContext
     std::shared_ptr<const document::Document> _doc;
 
 public:
-    PutDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted,
-                   std::shared_ptr<const document::Document> doc,
-                   uint32_t lid);
+    PutDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted,
+                   std::shared_ptr<const document::Document> doc, uint32_t lid);
     ~PutDoneContext() override;
 
     void registerPutLid(DocIdLimit *docIdLimit) { _docIdLimit = docIdLimit; }
diff --git a/searchcore/src/vespa/searchcore/proton/server/removedonecontext.cpp b/searchcore/src/vespa/searchcore/proton/server/removedonecontext.cpp
index b73b3c02120..5f98db0ee49 100644
--- a/searchcore/src/vespa/searchcore/proton/server/removedonecontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/removedonecontext.cpp
@@ -7,9 +7,8 @@
 
 namespace proton {
 
-RemoveDoneContext::RemoveDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor,
-                                     IDocumentMetaStore &documentMetaStore,
-                                     uint32_t lid)
+RemoveDoneContext::RemoveDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted,
+                                     vespalib::Executor &executor, IDocumentMetaStore &documentMetaStore, uint32_t lid)
     : OperationDoneContext(std::move(token)),
       _executor(executor),
       _task(),
diff --git a/searchcore/src/vespa/searchcore/proton/server/removedonecontext.h b/searchcore/src/vespa/searchcore/proton/server/removedonecontext.h
index 5eea6f7faaf..6aa5a2297d0 100644
--- a/searchcore/src/vespa/searchcore/proton/server/removedonecontext.h
+++ b/searchcore/src/vespa/searchcore/proton/server/removedonecontext.h
@@ -5,8 +5,6 @@
 #include "operationdonecontext.h"
 #include <vespa/searchcore/proton/common/ipendinglidtracker.h>
 #include <vespa/vespalib/util/executor.h>
-#include <vespa/document/base/globalid.h>
-#include <vespa/searchlib/common/serialnum.h>
 
 namespace proton {
 
@@ -28,8 +26,8 @@ class RemoveDoneContext : public OperationDoneContext
     IPendingLidTracker::Token _uncommitted;
 
 public:
-    RemoveDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor, IDocumentMetaStore &documentMetaStore,
-                      uint32_t lid);
+    RemoveDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor,
+                      IDocumentMetaStore &documentMetaStore, uint32_t lid);
     ~RemoveDoneContext() override;
 };
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
index df9ff54e29e..04ad4ee20ed 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
@@ -43,39 +43,11 @@ namespace proton {
 
 namespace {
 
-class PutDoneContextForMove : public PutDoneContext {
-private:
-    IDestructorCallback::SP _moveDoneCtx;
-
-public:
-    PutDoneContextForMove(FeedToken token, IPendingLidTracker::Token uncommitted,
-                          std::shared_ptr<const Document> doc, uint32_t lid, IDestructorCallback::SP moveDoneCtx)
-        : PutDoneContext(std::move(token), std::move(uncommitted),std::move(doc), lid),
-          _moveDoneCtx(std::move(moveDoneCtx))
-    {}
-    ~PutDoneContextForMove() override = default;
-};
-
-std::shared_ptr<PutDoneContext>
-createPutDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted,
-                     std::shared_ptr<const Document> doc, uint32_t lid, IDestructorCallback::SP moveDoneCtx)
-{
-    std::shared_ptr<PutDoneContext> result;
-    if (moveDoneCtx) {
-        result = std::make_shared<PutDoneContextForMove>(std::move(token), std::move(uncommitted),
-                                                         std::move(doc), lid, std::move(moveDoneCtx));
-    } else {
-        result = std::make_shared<PutDoneContext>(std::move(token), std::move(uncommitted), std::move(doc), lid);
-    }
-    return result;
-}
-
 std::shared_ptr<PutDoneContext>
-createPutDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted,
+createPutDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted,
                      std::shared_ptr<const Document> doc, uint32_t lid)
 {
-    return createPutDoneContext(std::move(token), std::move(uncommitted), std::move(doc),
-                                lid, IDestructorCallback::SP());
+    return std::make_shared<PutDoneContext>(std::move(token), std::move(uncommitted), std::move(doc), lid);
 }
 
 std::shared_ptr<UpdateDoneContext>
@@ -94,30 +66,11 @@ void setPrev(DocumentOperation &op, const documentmetastore::IStore::Result &res
     }
 }
 
-class RemoveDoneContextForMove : public RemoveDoneContext {
-private:
-    IDestructorCallback::SP _moveDoneCtx;
-
-public:
-    RemoveDoneContextForMove(FeedToken token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor,
-                             IDocumentMetaStore &documentMetaStore, uint32_t lid, IDestructorCallback::SP moveDoneCtx)
-        : RemoveDoneContext(std::move(token), std::move(uncommitted), executor, documentMetaStore, lid),
-          _moveDoneCtx(std::move(moveDoneCtx))
-    {}
-    ~RemoveDoneContextForMove() override = default;
-};
-
 std::shared_ptr<RemoveDoneContext>
-createRemoveDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor,
-                        IDocumentMetaStore &documentMetaStore, uint32_t lid, IDestructorCallback::SP moveDoneCtx)
+createRemoveDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted, vespalib::Executor &executor,
+                        IDocumentMetaStore &documentMetaStore, uint32_t lid)
 {
-    if (moveDoneCtx) {
-        return std::make_shared<RemoveDoneContextForMove>
-            (std::move(token), std::move(uncommitted), executor, documentMetaStore, lid, std::move(moveDoneCtx));
-    } else {
-        return std::make_shared<RemoveDoneContext>
-            (std::move(token), std::move(uncommitted), executor, documentMetaStore, lid);
-    }
+    return std::make_shared<RemoveDoneContext>(std::move(token), std::move(uncommitted), executor, documentMetaStore, lid);
 }
 
 class SummaryPutDoneContext : public OperationDoneContext
@@ -310,8 +263,7 @@ StoreOnlyFeedView::internalPut(FeedToken token, const PutOperation &putOp)
     if (docAlreadyExists && putOp.changedDbdId()) {
         //TODO, better to have an else than an assert ?
         assert(!putOp.getValidDbdId(_params._subDbId));
-        internalRemove(std::move(token), _pendingLidsForCommit->produce(putOp.getPrevLid()), serialNum,
-                       putOp.getPrevLid(), IDestructorCallback::SP());
+        internalRemove(std::move(token), _pendingLidsForCommit->produce(putOp.getPrevLid()), serialNum, putOp.getPrevLid());
     }
 }
 
@@ -593,8 +545,7 @@ StoreOnlyFeedView::internalRemove(FeedToken token, const RemoveOperationWithDocI
         if (rmOp.changedDbdId()) {
             //TODO Prefer else over assert ?
             assert(!rmOp.getValidDbdId(_params._subDbId));
-            internalRemove(std::move(token), _pendingLidsForCommit->produce(rmOp.getPrevLid()), serialNum,
-                           rmOp.getPrevLid(), IDestructorCallback::SP());
+            internalRemove(std::move(token), _pendingLidsForCommit->produce(rmOp.getPrevLid()), serialNum, rmOp.getPrevLid());
         }
     }
 }
@@ -612,19 +563,17 @@ StoreOnlyFeedView::internalRemove(FeedToken token, const RemoveOperationWithGid
     if (rmOp.getValidPrevDbdId(_params._subDbId)) {
         if (rmOp.changedDbdId()) {
             assert(!rmOp.getValidDbdId(_params._subDbId));
-            internalRemove(std::move(token), _pendingLidsForCommit->produce(rmOp.getPrevLid()), serialNum,
-                           rmOp.getPrevLid(), IDestructorCallback::SP());
+            internalRemove(std::move(token), _pendingLidsForCommit->produce(rmOp.getPrevLid()), serialNum, rmOp.getPrevLid());
         }
     }
 }
 
 void
-StoreOnlyFeedView::internalRemove(FeedToken token, IPendingLidTracker::Token uncommitted, SerialNum serialNum,
-                                  Lid lid, IDestructorCallback::SP moveDoneCtx)
+StoreOnlyFeedView::internalRemove(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted, SerialNum serialNum, Lid lid)
 {
     bool explicitReuseLid = _lidReuseDelayer.delayReuse(lid);
     auto onWriteDone = createRemoveDoneContext(std::move(token), std::move(uncommitted), _writeService.master(), _metaStore,
-                                               (explicitReuseLid ? lid : 0u), std::move(moveDoneCtx));
+                                               (explicitReuseLid ? lid : 0u));
     removeSummary(serialNum, lid, onWriteDone);
     removeAttributes(serialNum, lid, onWriteDone);
     removeIndexedFields(serialNum, lid, onWriteDone);
@@ -773,14 +722,13 @@ StoreOnlyFeedView::handleMove(const MoveOperation &moveOp, IDestructorCallback::
         if (moveOp.changedDbdId() && useDocumentMetaStore(serialNum)) {
             _gidToLidChangeHandler.notifyPut(FeedToken(), docId.getGlobalId(), moveOp.getLid(), serialNum);
         }
-        auto onWriteDone = createPutDoneContext(FeedToken(), _pendingLidsForCommit->produce(moveOp.getLid()),
-                                                doc, moveOp.getLid(), doneCtx);
+        auto onWriteDone = createPutDoneContext(doneCtx, _pendingLidsForCommit->produce(moveOp.getLid()), doc, moveOp.getLid());
         putSummary(serialNum, moveOp.getLid(), doc, onWriteDone);
         putAttributes(serialNum, moveOp.getLid(), *doc, onWriteDone);
         putIndexedFields(serialNum, moveOp.getLid(), doc, onWriteDone);
     }
     if (docAlreadyExists && moveOp.changedDbdId()) {
-        internalRemove(FeedToken(), _pendingLidsForCommit->produce(moveOp.getPrevLid()), serialNum, moveOp.getPrevLid(), doneCtx);
+        internalRemove(std::move(doneCtx), _pendingLidsForCommit->produce(moveOp.getPrevLid()), serialNum, moveOp.getPrevLid());
     }
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
index 7f1876cbbdf..5a0257fda57 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
@@ -53,7 +53,7 @@ public:
     using LidVector = LidVectorContext::LidVector;
     using Document = document::Document;
     using DocumentUpdate = document::DocumentUpdate;
-    using OnWriteDoneType =const std::shared_ptr<vespalib::IDestructorCallback> &;
+    using OnWriteDoneType = const std::shared_ptr<vespalib::IDestructorCallback> &;
     using OnForceCommitDoneType =const std::shared_ptr<ForceCommitContext> &;
     using OnOperationDoneType = const std::shared_ptr<OperationDoneContext> &;
     using OnPutDoneType = const std::shared_ptr<PutDoneContext> &;
@@ -66,6 +66,7 @@ public:
     using DocumentSP = std::shared_ptr<Document>;
     using DocumentUpdateSP = std::shared_ptr<DocumentUpdate>;
     using LidReuseDelayer = documentmetastore::LidReuseDelayer;
+    using IDestructorCallbackSP = std::shared_ptr<vespalib::IDestructorCallback>;
 
     using Lid = search::DocumentIdT;
 
@@ -180,8 +181,7 @@ private:
     // returns the number of documents removed.
     size_t removeDocuments(const RemoveDocumentsOperation &op, bool remove_index_and_attribute_fields);
 
-    void internalRemove(FeedToken token, IPendingLidTracker::Token uncommitted, SerialNum serialNum,
-                        Lid lid, std::shared_ptr<vespalib::IDestructorCallback> moveDoneCtx);
+    void internalRemove(IDestructorCallbackSP token, IPendingLidTracker::Token uncommitted, SerialNum serialNum, Lid lid);
 
     IPendingLidTracker::Token get_pending_lid_token(const DocumentOperation &op);
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.cpp b/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.cpp
index 3feaba15c00..8558c19f1ba 100644
--- a/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.cpp
@@ -7,7 +7,7 @@ using document::Document;
 
 namespace proton {
 
-UpdateDoneContext::UpdateDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted, const document::DocumentUpdate::SP &upd)
+UpdateDoneContext::UpdateDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted, const document::DocumentUpdate::SP &upd)
     : OperationDoneContext(std::move(token)),
       _uncommitted(std::move(uncommitted)),
       _upd(upd),
diff --git a/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.h b/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.h
index 6dad929aa26..6ca0e118b5b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.h
+++ b/searchcore/src/vespa/searchcore/proton/server/updatedonecontext.h
@@ -24,7 +24,7 @@ class UpdateDoneContext : public OperationDoneContext
     document::DocumentUpdate::SP _upd;
     std::shared_future<std::unique_ptr<const document::Document>> _doc;
 public:
-    UpdateDoneContext(FeedToken token, IPendingLidTracker::Token uncommitted, const document::DocumentUpdate::SP &upd);
+    UpdateDoneContext(IDestructorCallback::SP token, IPendingLidTracker::Token uncommitted, const document::DocumentUpdate::SP &upd);
     ~UpdateDoneContext() override;
 
     const document::DocumentUpdate &getUpdate() { return *_upd; }