Mercurial > hg > orthanc
changeset 2846:d386abc18133
simplification in SplitStudyJob, fix possible memory leak
author | Sebastien Jodogne <s.jodogne@gmail.com> |
---|---|
date | Fri, 28 Sep 2018 18:36:20 +0200 |
parents | 218e2c864d1d |
children | 2da68edacab6 |
files | Core/DicomParsing/DicomDirWriter.cpp Core/DicomParsing/DicomDirWriter.h Core/DicomParsing/ParsedDicomFile.cpp Core/DicomParsing/ParsedDicomFile.h OrthancServer/ServerJobs/SplitStudyJob.cpp OrthancServer/ServerJobs/SplitStudyJob.h |
diffstat | 6 files changed, 59 insertions(+), 102 deletions(-) [+] |
line wrap: on
line diff
--- a/Core/DicomParsing/DicomDirWriter.cpp Fri Sep 28 17:59:44 2018 +0200 +++ b/Core/DicomParsing/DicomDirWriter.cpp Fri Sep 28 18:36:20 2018 +0200 @@ -504,14 +504,6 @@ { } - DicomDirWriter::~DicomDirWriter() - { - if (pimpl_) - { - delete pimpl_; - } - } - void DicomDirWriter::SetUtcUsed(bool utc) { pimpl_->SetUtcUsed(utc);
--- a/Core/DicomParsing/DicomDirWriter.h Fri Sep 28 17:59:44 2018 +0200 +++ b/Core/DicomParsing/DicomDirWriter.h Fri Sep 28 18:36:20 2018 +0200 @@ -43,13 +43,11 @@ { private: class PImpl; - PImpl* pimpl_; + boost::shared_ptr<PImpl> pimpl_; public: DicomDirWriter(); - ~DicomDirWriter(); - void SetUtcUsed(bool utc); bool IsUtcUsed() const;
--- a/Core/DicomParsing/ParsedDicomFile.cpp Fri Sep 28 17:59:44 2018 +0200 +++ b/Core/DicomParsing/ParsedDicomFile.cpp Fri Sep 28 18:36:20 2018 +0200 @@ -944,59 +944,48 @@ void ParsedDicomFile::CreateFromDicomMap(const DicomMap& source, Encoding defaultEncoding) { - try - { - pimpl_->file_.reset(new DcmFileFormat); + pimpl_->file_.reset(new DcmFileFormat); + + const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET); - const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET); + if (tmp == NULL) + { + SetEncoding(defaultEncoding); + } + else if (tmp->IsBinary()) + { + LOG(ERROR) << "Invalid binary string in the SpecificCharacterSet (0008,0005) tag"; + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + else if (tmp->IsNull() || + tmp->GetContent().empty()) + { + SetEncoding(defaultEncoding); + } + else + { + Encoding encoding; - if (tmp == NULL) + if (GetDicomEncoding(encoding, tmp->GetContent().c_str())) { - SetEncoding(defaultEncoding); - } - else if (tmp->IsBinary()) - { - LOG(ERROR) << "Invalid binary string in the SpecificCharacterSet (0008,0005) tag"; - throw OrthancException(ErrorCode_ParameterOutOfRange); - } - else if (tmp->IsNull() || - tmp->GetContent().empty()) - { - SetEncoding(defaultEncoding); + SetEncoding(encoding); } else { - Encoding encoding; - - if (GetDicomEncoding(encoding, tmp->GetContent().c_str())) - { - SetEncoding(encoding); - } - else - { - LOG(ERROR) << "Unsupported value for the SpecificCharacterSet (0008,0005) tag: \"" - << tmp->GetContent() << "\""; - throw OrthancException(ErrorCode_ParameterOutOfRange); - } - } - - for (DicomMap::Map::const_iterator - it = source.map_.begin(); it != source.map_.end(); ++it) - { - if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET && - !it->second->IsNull()) - { - ReplacePlainString(it->first, it->second->GetContent()); - } + LOG(ERROR) << "Unsupported value for the SpecificCharacterSet (0008,0005) tag: \"" + << tmp->GetContent() << "\""; + throw OrthancException(ErrorCode_ParameterOutOfRange); } } - catch (OrthancException&) + + for (DicomMap::Map::const_iterator + it = source.map_.begin(); it != source.map_.end(); ++it) { - // Manually delete the PImpl to avoid a memory leak due to - // throwing the exception in the constructor - delete pimpl_; - pimpl_ = NULL; - throw; + if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET && + !it->second->IsNull()) + { + ReplacePlainString(it->first, it->second->GetContent()); + } } } @@ -1061,12 +1050,6 @@ } - ParsedDicomFile::~ParsedDicomFile() - { - delete pimpl_; - } - - DcmFileFormat& ParsedDicomFile::GetDcmtkObject() const { return *pimpl_->file_.get();
--- a/Core/DicomParsing/ParsedDicomFile.h Fri Sep 28 17:59:44 2018 +0200 +++ b/Core/DicomParsing/ParsedDicomFile.h Fri Sep 28 18:36:20 2018 +0200 @@ -63,6 +63,8 @@ # include "../RestApi/RestApiOutput.h" #endif +#include <boost/shared_ptr.hpp> + class DcmDataset; class DcmFileFormat; @@ -73,7 +75,7 @@ { private: struct PImpl; - PImpl* pimpl_; + boost::shared_ptr<PImpl> pimpl_; ParsedDicomFile(ParsedDicomFile& other, bool keepSopInstanceUid); @@ -108,8 +110,6 @@ ParsedDicomFile(DcmFileFormat& dicom); - ~ParsedDicomFile(); - DcmFileFormat& GetDcmtkObject() const; ParsedDicomFile* Clone(bool keepSopInstanceUid);
--- a/OrthancServer/ServerJobs/SplitStudyJob.cpp Fri Sep 28 17:59:44 2018 +0200 +++ b/OrthancServer/ServerJobs/SplitStudyJob.cpp Fri Sep 28 18:36:20 2018 +0200 @@ -85,25 +85,16 @@ /** * Chose the target UIDs **/ - - std::string series; - if (!modified->GetTagValue(series, DICOM_TAG_SERIES_INSTANCE_UID)) + + assert(modified->GetHasher().HashStudy() == sourceStudy_); + + std::string series = modified->GetHasher().HashSeries(); + + SeriesUidMap::const_iterator targetSeriesUid = seriesUidMap_.find(series); + + if (targetSeriesUid == seriesUidMap_.end()) { throw OrthancException(ErrorCode_BadFileFormat); // Should never happen - } - - std::string targetSeriesUid; - SeriesUidMap::const_iterator found = targetSeries_.find(series); - - if (found == targetSeries_.end()) - { - // Choose a random SeriesInstanceUID for this series - targetSeriesUid = FromDcmtkBridge::GenerateUniqueIdentifier(ResourceType_Series); - targetSeries_[series] = targetSeriesUid; - } - else - { - targetSeriesUid = found->second; } @@ -129,7 +120,7 @@ **/ modified->ReplacePlainString(DICOM_TAG_STUDY_INSTANCE_UID, targetStudyUid_); - modified->ReplacePlainString(DICOM_TAG_SERIES_INSTANCE_UID, targetSeriesUid); + modified->ReplacePlainString(DICOM_TAG_SERIES_INSTANCE_UID, targetSeriesUid->second); if (targetStudy_.empty()) { @@ -186,15 +177,6 @@ LOG(ERROR) << "Cannot split unknown study: " << sourceStudy; throw OrthancException(ErrorCode_UnknownResource); } - - std::list<std::string> children; - context_.GetIndex().GetChildren(children, sourceStudy); - - for (std::list<std::string>::const_iterator - it = children.begin(); it != children.end(); ++it) - { - sourceSeries_.insert(*it); - } } @@ -219,17 +201,23 @@ void SplitStudyJob::AddSourceSeries(const std::string& series) { + std::string parent; + if (IsStarted()) { throw OrthancException(ErrorCode_BadSequenceOfCalls); } - else if (sourceSeries_.find(series) == sourceSeries_.end()) + else if (!context_.GetIndex().LookupParent(parent, series, ResourceType_Study) || + parent != sourceStudy_) { LOG(ERROR) << "This series does not belong to the study to be split: " << series; throw OrthancException(ErrorCode_UnknownResource); } else { + // Generate a target SeriesInstanceUID for this series + seriesUidMap_[series] = FromDcmtkBridge::GenerateUniqueIdentifier(ResourceType_Series); + // Add all the instances of the series as to be processed std::list<std::string> instances; context_.GetIndex().GetChildren(instances, series); @@ -292,12 +280,11 @@ } + static const char* KEEP_SOURCE = "KeepSource"; static const char* SOURCE_STUDY = "SourceStudy"; - static const char* SOURCE_SERIES = "SourceSeries"; - static const char* KEEP_SOURCE = "KeepSource"; static const char* TARGET_STUDY = "TargetStudy"; static const char* TARGET_STUDY_UID = "TargetStudyUID"; - static const char* TARGET_SERIES = "TargetSeries"; + static const char* SERIES_UID_MAP = "SeriesUIDMap"; static const char* ORIGIN = "Origin"; static const char* REPLACEMENTS = "Replacements"; static const char* REMOVALS = "Removals"; @@ -318,10 +305,9 @@ keepSource_ = SerializationToolbox::ReadBoolean(serialized, KEEP_SOURCE); sourceStudy_ = SerializationToolbox::ReadString(serialized, SOURCE_STUDY); - SerializationToolbox::ReadSetOfStrings(sourceSeries_, serialized, SOURCE_SERIES); targetStudy_ = SerializationToolbox::ReadString(serialized, TARGET_STUDY); targetStudyUid_ = SerializationToolbox::ReadString(serialized, TARGET_STUDY_UID); - SerializationToolbox::ReadMapOfStrings(targetSeries_, serialized, TARGET_SERIES); + SerializationToolbox::ReadMapOfStrings(seriesUidMap_, serialized, SERIES_UID_MAP); origin_ = DicomInstanceOrigin(serialized[ORIGIN]); SerializationToolbox::ReadMapOfTags(replacements_, serialized, REPLACEMENTS); SerializationToolbox::ReadSetOfTags(removals_, serialized, REMOVALS); @@ -338,10 +324,9 @@ { target[KEEP_SOURCE] = keepSource_; target[SOURCE_STUDY] = sourceStudy_; - SerializationToolbox::WriteSetOfStrings(target, sourceSeries_, SOURCE_SERIES); target[TARGET_STUDY] = targetStudy_; target[TARGET_STUDY_UID] = targetStudyUid_; - SerializationToolbox::WriteMapOfStrings(target, targetSeries_, TARGET_SERIES); + SerializationToolbox::WriteMapOfStrings(target, seriesUidMap_, SERIES_UID_MAP); origin_.Serialize(target[ORIGIN]); SerializationToolbox::WriteMapOfTags(target, replacements_, REPLACEMENTS); SerializationToolbox::WriteSetOfTags(target, removals_, REMOVALS);
--- a/OrthancServer/ServerJobs/SplitStudyJob.h Fri Sep 28 17:59:44 2018 +0200 +++ b/OrthancServer/ServerJobs/SplitStudyJob.h Fri Sep 28 18:36:20 2018 +0200 @@ -47,13 +47,12 @@ ServerContext& context_; + std::set<DicomTag> allowedTags_; bool keepSource_; std::string sourceStudy_; - std::set<std::string> sourceSeries_; std::string targetStudy_; std::string targetStudyUid_; - SeriesUidMap targetSeries_; - std::set<DicomTag> allowedTags_; + SeriesUidMap seriesUidMap_; DicomInstanceOrigin origin_; Replacements replacements_; std::set<DicomTag> removals_;