Commit a46583d8 authored by Ian Humphrey's avatar Ian Humphrey
Browse files

Prevent a crash/segfault from occuring when opening too many cubes. Fixes #1951

git-svn-id: http://subversion.wr.usgs.gov/repos/prog/isis3/trunk@6325 41f8697f-d340-4b68-9986-7bafba869bb8
parent 1379b8e8
Loading
Loading
Loading
Loading
+33 −7
Original line number Diff line number Diff line
#include "CubeManager.h"

#include <iostream>
#include <sys/resource.h>

#include "Cube.h"
#include "CubeAttribute.h"
#include "CubeManager.h"
#include "FileName.h"
#include "IString.h"

#include <iostream>

namespace Isis {
  CubeManager CubeManager::p_instance;

@@ -14,9 +17,22 @@ namespace Isis {
   *
   */
  CubeManager::CubeManager() {
    p_minimumCubes = 0;

    // Get the maximum allowable number of open files for a process
    struct rlimit fileLimit;
    
    if (getrlimit(RLIMIT_NOFILE, &fileLimit) != 0) {
      QString msg = "Cannot read the maximum allowable open files from system resources.";
      throw IException(IException::Programmer, msg, _FILEINFO_);
    }
    
    // Allow for library files, etc used by this process
    // So set or file limit to 60% of maximum allowed number of opened files
    p_maxOpenFiles = fileLimit.rlim_cur * .60;
    p_currentLimit = p_maxOpenFiles;
  }


  /**
   * This is the CubeManager destructor. This method calls CleanCubes().
   *
@@ -25,6 +41,7 @@ namespace Isis {
    CleanCubes();
  }


  /**
   * This method opens a cube. If the cube is already opened, this method will
   * return the cube from memory. The CubeManager class retains ownership of this
@@ -33,6 +50,11 @@ namespace Isis {
   * fail. This method does not guarantee you are the only one with this pointer,
   * nor is it recommended to keep this pointer out of a local (method) scope.
   *
   * Note that this method will allow for 60% of the system's maximum file limit + 1
   * cubes to be opened, since the cube passed is opened and then cleanup follows. However,
   * since our maximum limit is 60% of the system limit, this will allow enough room for
   * this extra file to be opened before the cleanup occurs. 
   *
   * @param cubeFileName The filename of the cube you wish to open
   *
   * @return Cube* A pointer to the cube object that CubeManager retains ownership
@@ -58,6 +80,9 @@ namespace Isis {
      // Bands are the only thing input attributes can affect
      (*searchResult)->setVirtualBands(attIn.bands());

      // Need to clean up memory if there is a problem opening a cube
      // This allows the CubeManager class to clean up the dynamically alloc'd
      // Cube before rethrowing the exception from Cube's open method 
      try {
        (*searchResult)->open(fileName);
      }
@@ -71,17 +96,16 @@ namespace Isis {
    p_opened.removeAll(fileName);
    p_opened.enqueue(fileName);

    // cleanup if necessary
    if(p_minimumCubes != 0) {
      while(p_opened.size() > (int)(p_minimumCubes)) {
    // cleanup excess cubes
    while (p_opened.size() > (int)(p_currentLimit)) {
      QString needsCleaned = p_opened.dequeue();
      CleanCubes(needsCleaned);
    }
    }

    return (*searchResult);
  }


  /**
   * This method removes a cube from memory, if it exists. If the cube is not
   * loaded into memory, nothing happens. This will cause any pointers to this
@@ -90,6 +114,7 @@ namespace Isis {
   * @param cubeFileName The filename of the cube to remove from memory
   */
  void CubeManager::CleanCubes(const QString &cubeFileName) {

    QString fileName(FileName(cubeFileName).expanded());
    QMap<QString, Cube *>::iterator searchResult = p_cubes.find(fileName);

@@ -102,6 +127,7 @@ namespace Isis {
    p_cubes.erase(searchResult);
  }


  /**
   * This method removes all cubes from memory. All pointers returned via OpenCube
   * will be invalid.
+31 −4
Original line number Diff line number Diff line
#ifndef CubeManager_h
#define CubeManager_h

#include <QString>
#include "IException.h"

#include <QMap>
#include <QQueue>
#include <QString>

/*
 *   Unless noted otherwise, the portions of Isis written by the
@@ -47,12 +49,19 @@ namespace Isis {
   *            cube attributes (input) when opening cubes.
   *   @history 2011-06-08 Steven Lambright - Better handles the case when a
   *                Cube fails to open, fixes #161.
   *   @history 2015-07-15 Ian Humphrey - Added private member variable to store the max number
   *                           files opened (60% of the system's limitations). Modified 
   *                           SetNumOpenCubes to set the maximum number of open cubes to
   *                           60% of the system's file limits if the passed value exceeds this
   *                           60% limitations. Modified OpenCube to always clean excess cubes. 
   *                           Updated unit test for better test coverage. Fixes #1951. 
   */
  class CubeManager  {
    public:
      CubeManager();
      ~CubeManager();


      /**
       * This method calls the method OpenCube() on the static instance
       *
@@ -66,6 +75,7 @@ namespace Isis {
        return p_instance.OpenCube(cubeFileName);
      }


      /**
       * This method calls CleanCubes(const QString &cubeFileName)  on the static
       * instance
@@ -78,6 +88,7 @@ namespace Isis {
        p_instance.CleanCubes(cubeFileName);
      }


      /**
       * This method calls CleanCubes() on the static instance
       *
@@ -87,33 +98,49 @@ namespace Isis {
        p_instance.CleanCubes();
      };


      void CleanCubes(const QString &cubeFileName);
      void CleanCubes();


      Cube *OpenCube(const QString &cubeFileName);


      /**
       * This sets the maximum number of opened cubes for this instance of
       * CubeManager. The last "maxCubes" opened cubes are guaranteed to be
       * valid as long as one of the CleanCubes(...) are not called.
       * If the maximum number of open cubes specified exceeds 60% of system limitations,
       * the maximum number of opened cubes will be set to a 60% of the 
       * system's open file limitation (this considers files used by the current process).
       *
       * @param numCubes Maximum number of open cubes
       */
      void SetNumOpenCubes(unsigned int numCubes) {
        p_minimumCubes = numCubes;

        // check to see if numCubes exceeds the number of open files limitations
        if (numCubes > p_maxOpenFiles) {
          p_currentLimit = p_maxOpenFiles;
        }
        p_currentLimit = numCubes;
      }


    protected:
      //! There is always at least one instance of CubeManager around
      static CubeManager p_instance;

      //! This keeps track of the open cubes
      QMap<QString, Cube *> p_cubes;

      //! This keeps track of cubes that have been opened
      QQueue<QString> p_opened;

      //! At least this many cubes must be allowed in memory, more can be cleaned up, 0 means no limit
      unsigned int p_minimumCubes;
      //! The current limit regarding number of open files allowed
      unsigned int p_currentLimit;

      //! 60% of the maximum number of open files allowed by system resources
      unsigned int p_maxOpenFiles;
  };
}

+34 −0
Original line number Diff line number Diff line
@@ -25,3 +25,37 @@ Verify cube attributes have been taken into account
  Cube 4 | 1
  Cube 5 | 1
  Cube 6 | 1

Verify blobTruth cubes have been cleaned
Cube FileNames: 
  1 : isisTruth
  2 : isisTruth
  3 : 
  4 : 
  5 : isisTruth
  6 : isisTruth

Verify remaining cubes have been cleaned
Cube FileNames: 
  1 : 
  2 : 
  3 : 
  4 : 
  5 : 
  6 : 

Set number of open cubes to 2

Currently managed cubes:
  1 : isisTruth
  2 : blobTruth

Opened isisTruth2.cub.
Verify isisTruth2.cub is now managed and limit of 2 is enforced:
  1 : blobTruth
  2 : isisTruth2

Setting number of open cubes > 60 percent of system open file limit

Attempting to open a file that does not exist:
  **I/O ERROR** Unable to open [dne.cub].
+76 −0
Original line number Diff line number Diff line
@@ -63,4 +63,80 @@ int main(int argc, char *argv[]) {
  for(int i = 0; i < (int)cubes.size(); i++) {
    std::cout << "  Cube " << i + 1 << " | " << cubes[i]->bandCount() << std::endl;
  }
  std::cout << std::endl;

  // Test CleanUp() methods
 
  // Try cleaning up a cube that isn't managed by CubeManager
  CubeManager::CleanUp("unmanagedCube.cub");

  // Clean up one of the managed cubes
  CubeManager::CleanUp("$base/testData/blobTruth.cub");

  // Print Cube FileNames to verify that we have cleaned isisTruth cubes correctly
  std::cout << "Verify blobTruth cubes have been cleaned" << std::endl;
  std::cout << "Cube FileNames: " << std::endl;

  for (int i = 0; i < (int)cubes.size(); i++) {
    std::cout << "  " << i + 1 << " : " << FileName(cubes[i]->fileName()).baseName() << std::endl;
  }
  std::cout << std::endl;

  // Clean up remaining cubes
  CubeManager::CleanUp();

  // Print Cube FileNames to verify that we have cleaned remaining cubes correctly
  std::cout << "Verify remaining cubes have been cleaned" << std::endl;
  std::cout << "Cube FileNames: " << std::endl;

  for (int i = 0; i < (int)cubes.size(); i++) {
    std::cout << "  " << i + 1 << " : " << FileName(cubes[i]->fileName()).baseName() << std::endl;
  }
  std::cout << std::endl;

  // Create a CubeManager instance (implicitly test destructor)
  QVector<Cube *> cubes2;
  CubeManager mgr;
  cubes2.push_back(mgr.OpenCube("$base/testData/isisTruth.cub"));
  cubes2.push_back(mgr.OpenCube("$base/testData/blobTruth.cub"));

  // Test setting an opened cube limit
  mgr.SetNumOpenCubes(2);
  std::cout << "Set number of open cubes to 2" << std::endl;
  std::cout << endl;

  // Print Cube FileNames to verify the currently managed cubes
  std::cout << "Currently managed cubes:" << std::endl;
  for (int i = 0; i < (int)cubes2.size(); i++) {
    std::cout << "  " << i + 1 << " : " << FileName(cubes2[i]->fileName()).baseName() << std::endl;
  }
  std::cout << std::endl;

  // This will test the cleanup in OpenCube(const QString &)
  cubes2.push_back(mgr.OpenCube("$base/testData/isisTruth2.cub"));
  // we pop the front because OpenCube should dequeue and clean 1 item to enforce limit
  cubes2.pop_front();
  std::cout << "Opened isisTruth2.cub." << std::endl;
  std::cout << "Verify isisTruth2.cub is now managed and limit of 2 is enforced:" << std::endl;
  for (int i = 0; i < (int)cubes2.size(); i++) {
    std::cout << "  " << i + 1 << " : " << FileName(cubes2[i]->fileName()).baseName() << std::endl;
  }
  std::cout << std::endl;

  // Cleanup
  mgr.CleanCubes();

  // Set a open cube limit that exceeds the system open file limit
  std::cout << "Setting number of open cubes > 60 percent of system open file limit" << std::endl;
  mgr.SetNumOpenCubes(1000000);
  std::cout << std::endl;
  
  // Open a cube that DNE
  std::cout << "Attempting to open a file that does not exist:" << std::endl;
  try {
    mgr.OpenCube("dne.cub");
  } catch (IException &e) {
    std::cout << "  " << e.what() << std::endl;
  }

}