DQM protobuf reads and writes now close the file descriptors properly.

Well, not properly, this is just a quick fix. We should rewrite and avoid using ::open altogether.
dmitrijus · Nov 23, 2014 · cda6b95 · cda6b95
1 parent 30ab803
commit cda6b95
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git a/DQMServices/Components/bin/fastHadd.cc b/DQMServices/Components/bin/fastHadd.cc
@@ -205,6 +205,7 @@ void writeMessage(const dqmstorepb::ROOTFilePB &dqmstore_output_msg,
                       S_IRUSR | S_IWUSR |
                       S_IRGRP | S_IWGRP |
                       S_IROTH);
+
   FileOutputStream out_stream(out_fd);
   GzipOutputStream::Options options;
   options.format = GzipOutputStream::GZIP;
@@ -213,7 +214,10 @@ void writeMessage(const dqmstorepb::ROOTFilePB &dqmstore_output_msg,
                                options);
   dqmstore_output_msg.SerializeToZeroCopyStream(&gzip_stream);
 
-  google::protobuf::ShutdownProtobufLibrary();
+  // make sure we flush before close
+  gzip_stream.Close();
+  out_stream.file_stream.Close();
+  ::close(out_fd);
 }
 
 
@@ -303,7 +307,7 @@ int convertFile(const std::string &output_filename,
   DEBUG(0, "Converting file " << filenames[0] << std::endl);
   dqmstorepb::ROOTFilePB dqmstore_message;
 
-  int filedescriptor = open(filenames[0].c_str(), O_RDONLY);
+  int filedescriptor = ::open(filenames[0].c_str(), O_RDONLY);
   FileInputStream fin(filedescriptor);
   GzipInputStream input(&fin);
   CodedInputStream input_coded(&input);
@@ -313,6 +317,7 @@ int convertFile(const std::string &output_filename,
               << filenames[0] << std::endl;
     return ERR_NOFILE;
   }
+  ::close(filedescriptor);
 
   for (int i = 0; i < dqmstore_message.histo_size(); i++) {
     const dqmstorepb::ROOTFilePB::Histo& h = dqmstore_message.histo(i);
@@ -350,7 +355,6 @@ int convertFile(const std::string &output_filename,
     DEBUG(1, obj->GetName() << std::endl);
   }
   output.Close();
-  google::protobuf::ShutdownProtobufLibrary();
   return 0;
 }
 
@@ -360,7 +364,7 @@ int dumpFiles(const std::vector<std::string> &filenames) {
     DEBUG(0, "Dumping file " << filenames[i] << std::endl);
     dqmstorepb::ROOTFilePB dqmstore_message;
 
-    int filedescriptor = open(filenames[0].c_str(), O_RDONLY);
+    int filedescriptor = ::open(filenames[0].c_str(), O_RDONLY);
     FileInputStream fin(filedescriptor);
     GzipInputStream input(&fin);
     CodedInputStream input_coded(&input);
@@ -370,6 +374,7 @@ int dumpFiles(const std::vector<std::string> &filenames) {
                 << filenames[0] << std::endl;
       return ERR_NOFILE;
     }
+    ::close(filedescriptor);
 
     for (int i = 0; i < dqmstore_message.histo_size(); i++) {
       const dqmstorepb::ROOTFilePB::Histo& h = dqmstore_message.histo(i);
@@ -384,7 +389,7 @@ int dumpFiles(const std::vector<std::string> &filenames) {
       DEBUG(1, "Flags: " << h.flags() << std::endl);
     }
   }
-  google::protobuf::ShutdownProtobufLibrary();
+
   return 0;
 }
 
@@ -401,7 +406,7 @@ int addFiles(const std::string &output_filename,
   {
     dqmstorepb::ROOTFilePB dqmstore_message;
     int filedescriptor;
-    if ((filedescriptor = open(filenames[0].c_str(), O_RDONLY)) == -1) {
+    if ((filedescriptor = ::open(filenames[0].c_str(), O_RDONLY)) == -1) {
       std::cout << "Fatal Error opening file "
                 << filenames[0] << std::endl;
       return ERR_NOFILE;
@@ -416,6 +421,8 @@ int addFiles(const std::string &output_filename,
                 << filenames[0] << std::endl;
       return ERR_NOFILE;
     }
+    ::close(filedescriptor);
+
     for (int i = 0; i < dqmstore_message.histo_size(); i++) {
       std::string path;
       std::string objname;
@@ -438,7 +445,7 @@ int addFiles(const std::string &output_filename,
     DEBUG(1, "Adding file " << filenames[i] << std::endl);
     dqmstorepb::ROOTFilePB dqmstore_msg;
     int filedescriptor;
-    if ((filedescriptor = open(filenames[i].c_str(), O_RDONLY)) == -1) {
+    if ((filedescriptor = ::open(filenames[i].c_str(), O_RDONLY)) == -1) {
       std::cout << "Fatal Error opening file "
                 << filenames[i] << std::endl;
       return ERR_NOFILE;
@@ -452,6 +459,7 @@ int addFiles(const std::string &output_filename,
                 << filenames[0] << std::endl;
       return ERR_NOFILE;
     }
+    ::close(filedescriptor);
 
     std::set<MicroME>::iterator mi = micromes.begin();
     std::set<MicroME>::iterator me = micromes.end();
@@ -632,5 +640,7 @@ int main(int argc, char * argv[]) {
   else if (task == TASK_ENCODE)
     ret = encodeFile(output_file, filenames);
 
+
+  google::protobuf::ShutdownProtobufLibrary();
   return ret;
 }
diff --git a/DQMServices/Core/src/DQMStore.cc b/DQMServices/Core/src/DQMStore.cc
@@ -2534,6 +2534,11 @@ void DQMStore::savePB(const std::string &filename,
                                options);
   dqmstore_message.SerializeToZeroCopyStream(&gzip_stream);
 
+  // we need to flush it before we close the fd
+  gzip_stream.Close();
+  file_stream.Close();
+  ::close(filedescriptor);
+
   // Maybe make some noise.
   if (verbose_)
     std::cout << "DQMStore::savePB: successfully wrote " << nme
@@ -3064,6 +3069,7 @@ DQMStore::readFilePB(const std::string &filename,
     raiseDQMError("DQMStore", "Fatal parsing file '%s'", filename.c_str());
     return false;
   }
+  ::close(filedescriptor);
 
   for (int i = 0; i < dqmstore_message.histo_size(); i++) {
     std::string path;