diff options
10 files changed, 63 insertions, 113 deletions
diff --git a/filedistribution/src/tests/common/testCommon.cpp b/filedistribution/src/tests/common/testCommon.cpp index 699d3628547..1902d4ec7db 100644 --- a/filedistribution/src/tests/common/testCommon.cpp +++ b/filedistribution/src/tests/common/testCommon.cpp @@ -12,13 +12,13 @@ namespace fd = filedistribution; const size_t bufferCapacity = 10; -fd::Move<fd::Buffer> +fd::Buffer getBuffer() { const char* test = "test"; fd::Buffer buffer(test, test + strlen(test)); buffer.reserve(bufferCapacity); buffer.push_back(0); - return fd::move(buffer); + return buffer; } BOOST_AUTO_TEST_CASE(bufferTest) { diff --git a/filedistribution/src/vespa/filedistribution/common/buffer.h b/filedistribution/src/vespa/filedistribution/common/buffer.h index 2325b3e11ee..79927119e1f 100644 --- a/filedistribution/src/vespa/filedistribution/common/buffer.h +++ b/filedistribution/src/vespa/filedistribution/common/buffer.h @@ -5,37 +5,6 @@ namespace filedistribution { -struct USED_FOR_MOVING {}; - -template <class T> -class Move { - mutable T _holder; -public: - Move(T& toMove) - :_holder(USED_FOR_MOVING()) - { - _holder.swap(toMove); - } - - Move(const Move& other) - :_holder(USED_FOR_MOVING()) - { - _holder.swap(other._holder); - } - - void swap(T& t) const { - _holder.swap(t); - } - -private: - Move& operator=(const Move&); -}; - -template <class T> -inline Move<T> move(T& t) { - return Move<T>(t); -} - class Buffer { size_t _capacity; char* _buf; @@ -54,13 +23,15 @@ public: _size(0) {} - Buffer(const Move<Buffer>& buffer); - - explicit Buffer(USED_FOR_MOVING) - :_capacity(0), - _buf(0), - _size(0) - {} + Buffer(Buffer && rhs) : + _capacity(rhs._capacity), + _buf(rhs._buf), + _size(rhs._size) + { + rhs._capacity = 0; + rhs._size = 0; + rhs._buf = nullptr; + } template <typename ITER> Buffer(ITER beginIter, ITER endIter) @@ -75,13 +46,8 @@ public: delete[] _buf; } - size_t capacity() const { - return _capacity; - } - - size_t size() const { - return _size; - } + size_t capacity() const { return _capacity; } + size_t size() const { return _size; } //might expose uninitialized memory void resize(size_t newSize) { @@ -114,39 +80,14 @@ public: _buf[_size++] = c; } - iterator begin() { - return _buf; - } - - iterator end() { - return _buf + _size; - } - - const_iterator begin() const { - return _buf; - } - - const_iterator end() const { - return _buf + _size; - } - - char operator[](size_t i) const { - return _buf[i]; - } - - char& operator[](size_t i) { - return _buf[i]; - } + iterator begin() { return _buf; } + iterator end() { return _buf + _size; } + const_iterator begin() const { return _buf; } + const_iterator end() const { return _buf + _size; } + char operator[](size_t i) const { return _buf[i]; } + char& operator[](size_t i) { return _buf[i]; } }; -inline Buffer::Buffer(const Move<Buffer>& buffer) - :_capacity(0), - _buf(0), - _size(0) -{ - buffer.swap(*this); -} - } //namespace filedistribution diff --git a/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp b/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp index fd54b65cdbc..a8e7505cfed 100644 --- a/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp +++ b/filedistribution/src/vespa/filedistribution/manager/createtorrent.cpp @@ -66,14 +66,14 @@ CreateTorrent(const boost::filesystem::path& path) _entry(createEntry(_path)) {} -const filedistribution::Move<filedistribution::Buffer> +filedistribution::Buffer filedistribution:: CreateTorrent:: bencode() const { Buffer buffer(static_cast<int>(targetTorrentSize)); libtorrent::bencode(std::back_inserter(buffer), _entry); - return move(buffer); + return buffer; } const std::string diff --git a/filedistribution/src/vespa/filedistribution/manager/createtorrent.h b/filedistribution/src/vespa/filedistribution/manager/createtorrent.h index 93d56fa9e6f..ad1ad4379dd 100644 --- a/filedistribution/src/vespa/filedistribution/manager/createtorrent.h +++ b/filedistribution/src/vespa/filedistribution/manager/createtorrent.h @@ -15,7 +15,7 @@ class CreateTorrent { public: CreateTorrent(const boost::filesystem::path& path); - const Move<Buffer> bencode() const; + Buffer bencode() const; const std::string fileReference() const; }; diff --git a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h index d7803b8afea..b575a166701 100644 --- a/filedistribution/src/vespa/filedistribution/model/filedbmodel.h +++ b/filedistribution/src/vespa/filedistribution/model/filedbmodel.h @@ -43,7 +43,7 @@ public: virtual bool hasFile(const std::string& fileReference) = 0; virtual void addFile(const std::string& fileReference, const Buffer& buffer) = 0; - virtual Move<Buffer> getFile(const std::string& fileReference) = 0; + virtual Buffer getFile(const std::string& fileReference) = 0; virtual void cleanFiles(const std::vector<std::string>& filesToPreserve) = 0; virtual void setDeployedFilesToDownload(const std::string& hostName, diff --git a/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h b/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h index 169225deaf2..cdc77692017 100644 --- a/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h +++ b/filedistribution/src/vespa/filedistribution/model/mockfiledistributionmodel.h @@ -21,15 +21,14 @@ public: _fileReferences.push_back(fileReference); } - Move<Buffer> getFile(const std::string& fileReference) { + Buffer getFile(const std::string& fileReference) override { (void)fileReference; const char* resultStr = "result"; Buffer result(resultStr, resultStr + strlen(resultStr)); - return move(result); + return result; } - virtual void cleanFiles( - const std::vector<std::string> &) {} + virtual void cleanFiles(const std::vector<std::string> &) {} virtual void setDeployedFilesToDownload(const std::string&, @@ -50,8 +49,7 @@ public: return HostStatus(); } - Progress getProgress(const std::string&, - const std::vector<std::string>&) { + Progress getProgress(const std::string&, const std::vector<std::string>&) { return Progress(); } }; diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp index 4e93b0907ab..89ff72e4a36 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp +++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp @@ -21,7 +21,6 @@ typedef std::unique_lock<std::mutex> UniqueLock; using filedistribution::ZKFacade; -using filedistribution::Move; using filedistribution::Buffer; using filedistribution::ZKGenericException; using filedistribution::ZKException; @@ -333,7 +332,7 @@ ZKFacade::getString(const Path& path) { return std::string(buffer.begin(), buffer.end()); } -const Move<Buffer> +Buffer ZKFacade::getData(const Path& path) { RetryController retryController(this); Buffer buffer(_maxDataSize); @@ -349,34 +348,30 @@ ZKFacade::getData(const Path& path) { retryController.throwIfError(path); buffer.resize(bufferSize); - return move(buffer); + return buffer; } -const Move<Buffer> +Buffer ZKFacade::getData(const Path& path, const NodeChangedWatcherSP& watcher) { - void* watcherContext = registerWatcher(watcher); + RegistrationGuard unregisterGuard(*this, watcher); + void* watcherContext = unregisterGuard.get(); RetryController retryController(this); - try { - Buffer buffer(_maxDataSize); - int bufferSize = _maxDataSize; - - do { - Stat stat; - bufferSize = _maxDataSize; + Buffer buffer(_maxDataSize); + int bufferSize = _maxDataSize; - retryController( zoo_wget(_zhandle, path.string().c_str(), &ZKWatcher::watcherFn, watcherContext, &*buffer.begin(), &bufferSize, &stat)); - } while(retryController.shouldRetry()); + do { + Stat stat; + bufferSize = _maxDataSize; - retryController.throwIfError(path); + retryController( zoo_wget(_zhandle, path.string().c_str(), &ZKWatcher::watcherFn, watcherContext, &*buffer.begin(), &bufferSize, &stat)); + } while (retryController.shouldRetry()); - buffer.resize(bufferSize); - return move(buffer); + retryController.throwIfError(path); - } catch (const ZKException & e) { - unregisterWatcher(watcherContext); - throw; - } + buffer.resize(bufferSize); + unregisterGuard.release(); + return buffer; } void diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.h b/filedistribution/src/vespa/filedistribution/model/zkfacade.h index b6a3e42d899..7e6db4d381f 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfacade.h +++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.h @@ -90,9 +90,9 @@ public: bool hasNode(const Path&, const NodeChangedWatcherSP&); const std::string getString(const Path&); - const Move<Buffer> getData(const Path&); //throws ZKNodeDoesNotExistsException + Buffer getData(const Path&); //throws ZKNodeDoesNotExistsException //if watcher is specified, it will be set even if the node does not exists - const Move<Buffer> getData(const Path&, const NodeChangedWatcherSP&); //throws ZKNodeDoesNotExistsException + Buffer getData(const Path&, const NodeChangedWatcherSP&); //throws ZKNodeDoesNotExistsException //Parent path must exist void setData(const Path&, const Buffer& buffer, bool mustExist = false); @@ -116,6 +116,22 @@ public: } private: + class RegistrationGuard { + public: + RegistrationGuard & operator = (const RegistrationGuard &) = delete; + RegistrationGuard(const RegistrationGuard &) = delete; + RegistrationGuard(ZKFacade & zk, const NodeChangedWatcherSP & watcher) : _zk(zk), _watcherContext(_zk.registerWatcher(watcher)) { } + ~RegistrationGuard() { + if (_watcherContext) { + _zk.unregisterWatcher(_watcherContext); + } + } + void * get() { return _watcherContext; } + void release() { _watcherContext = nullptr; } + private: + ZKFacade & _zk; + void * _watcherContext; + }; void* registerWatcher(const NodeChangedWatcherSP &); //returns watcherContext std::shared_ptr<ZKWatcher> unregisterWatcher(void* watcherContext); void invokeWatcher(void* watcherContext); diff --git a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp index 70827305138..ab886a5889b 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp +++ b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.cpp @@ -58,7 +58,7 @@ ZKFileDBModel::addFile(const std::string& fileReference, const Buffer& buffer) { return _zk->setData(createPath(fileReference), buffer); } -Move<Buffer> +Buffer ZKFileDBModel::getFile(const std::string& fileReference) { try { return _zk->getData(createPath(fileReference)); diff --git a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h index 4249410c00e..fc3b088a385 100644 --- a/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h +++ b/filedistribution/src/vespa/filedistribution/model/zkfiledbmodel.h @@ -29,7 +29,7 @@ public: //overrides bool hasFile(const std::string& fileReference); void addFile(const std::string& fileReference, const Buffer& buffer); - Move<Buffer> getFile(const std::string& fileReference); + Buffer getFile(const std::string& fileReference) override; void cleanFiles(const std::vector<std::string>& filesToPreserve); void setDeployedFilesToDownload(const std::string& hostName, |