From be5e7a2874eeb79ff371b4f2f60f5a59d4f63f8d Mon Sep 17 00:00:00 2001 From: Eric Cano <Eric.Cano@cern.ch> Date: Tue, 1 Aug 2017 00:55:07 +0200 Subject: [PATCH] Fixed garbage collector mishandling disappearing agent. The appearance and disappearance of the agents is totally asynchronous for the garbage collector. For this reason, GC should tolerate deletion of object at any time. This commit fixes the case where the object disappears shortly after the garbage collector decides to watch it. --- objectstore/GarbageCollector.cpp | 62 ++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/objectstore/GarbageCollector.cpp b/objectstore/GarbageCollector.cpp index 6696ecb7f2..496ead135c 100644 --- a/objectstore/GarbageCollector.cpp +++ b/objectstore/GarbageCollector.cpp @@ -84,37 +84,47 @@ void GarbageCollector::aquireTargets(log::LogContext & lc) { // First, check that the agent entry exists, and that ownership // is indeed pointing to the agent register Agent ag(*c, m_objectStore); - if (!ag.exists()) { - // This is a dangling pointer to a dead object: - // remove it in the agentRegister. - m_agentRegister.removeAgent(*c); - continue; - } - ScopedExclusiveLock agLock(ag); - ag.fetch(); - // Check that the actual owner is the agent register. - // otherwise, it should not be listed as an agent to monitor - if (ag.getOwner() != m_agentRegister.getAddressIfSet()) { - m_agentRegister.trackAgent(ag.getAddressIfSet()); - agLock.release(); - continue; - } - // We are now interested in tracking this agent. So we will transfer its - // ownership. We alredy have an exclusive lock on the agent. - // Lock ours + ScopedExclusiveLock agLock; Agent ourAgent(m_ourAgentReference.getAgentAddress(), m_objectStore); - ScopedExclusiveLock oaLock(ourAgent); - ourAgent.fetch(); - ourAgent.addToOwnership(ag.getAddressIfSet()); - ourAgent.commit(); - // We now have a pointer to the agent, we can make the ownership official - ag.setOwner(ourAgent.getAddressIfSet()); - ag.commit(); + ScopedExclusiveLock oaLock; + try { + if (!ag.exists()) { + // This is a dangling pointer to a dead object: + // remove it in the agentRegister. + m_agentRegister.removeAgent(*c); + continue; + } + agLock.lock(ag); + ag.fetch(); + // Check that the actual owner is the agent register. + // otherwise, it should not be listed as an agent to monitor + if (ag.getOwner() != m_agentRegister.getAddressIfSet()) { + m_agentRegister.trackAgent(ag.getAddressIfSet()); + agLock.release(); + continue; + } + // We are now interested in tracking this agent. So we will transfer its + // ownership. We alredy have an exclusive lock on the agent. + // Lock ours + + oaLock.lock(ourAgent); + ourAgent.fetch(); + ourAgent.addToOwnership(ag.getAddressIfSet()); + ourAgent.commit(); + // We now have a pointer to the agent, we can make the ownership official + ag.setOwner(ourAgent.getAddressIfSet()); + ag.commit(); + } catch (cta::exception::Exception & ex) { + // We received an exception. This can happen is the agent disappears under our feet. + // This is fine, we just let go this time, and trimGoneTargets() will just de-reference + // it later. But if the object is present, we have a problem. + if (m_objectStore.exists(*c)) throw; + } log::ScopedParamContainer params(lc); params.add("agentAddress", ag.getAddressIfSet()) .add("gcAgentAddress", ourAgent.getAddressIfSet()); lc.log(log::INFO, "In GarbageCollector::aquireTargets(): started tracking an untracked agent"); - // Agent is officially our, we can remove it from the untracked agent's + // Agent is officially ours, we can remove it from the untracked agent's // list m_agentRegister.trackAgent(ag.getAddressIfSet()); m_agentRegister.commit(); -- GitLab