RC3: Bug in Paste related to deleted DSO

Ok this one is a little complex to understand. The assertion occurs only in debug builds... but it's something that only by chance doesn't cause a crash in release. The assertion:

Quote:
Fatal: (c:\projects\kvk\engine\source\core\filestream.cc @ 187) FileStream::_read: the stream isn't open

This happens when i cut and paste in a debug build of TGB. It occurs because tools\levelEditor\data\clipboard.cs.dso is deleted after the ResourceManager enumerates it at startup. Then when when clipboard.cs is exec()'d as part of the paste operation, ResourceManager->find() happily returns a valid ResourceObject for a DSO which does not exist and causes this assertion when reading the DSO version number.

The correct fix seems to be in ResourceManager::openStream... it fails to check if the file was really opened:

Stream * ResManager::openStream (ResourceObject * obj)
{
   // if filename is not known, exit now
   if (!obj)
      return NULL;

   if (echoFileNames)
      Con::printf ("FILE ACCESS: %s/%s", obj->path, obj->name);

   // used for openStream stream access
   FileStream *diskStream = NULL;

   // if disk file
   if (obj->flags & (ResourceObject::File))
   {
      diskStream = new FileStream;
      diskStream->open (buildPath (obj->path, obj->name), FileStream::Read); // No check for success!
      obj->fileSize = diskStream->getStreamSize ();
      return diskStream;
   }

The initial fix is simple...

// if disk file
   if (obj->flags & (ResourceObject::File))
   {
      diskStream = new FileStream;
      if ( !diskStream->open (buildPath (obj->path, obj->name), FileStream::Read) )
      {
         delete diskStream;
         return NULL;
      }
      obj->fileSize = diskStream->getStreamSize ();
      return diskStream;
   }

This should have been the only fix necessary as every place in Torque should check the result of OpenStream. Well only one place in Torque is the return value not checked.... in ConsoleFunction(exec) exactly where our original bug was occurring...

// If we had a DSO, let's check to see if we should be reading from it.
   if(compiled && rCom && (!rScr || Platform::compareFileTimes(comModifyTime, scrModifyTime) >= 0))
   {
      compiledStream = ResourceManager->openStream(nameBuffer);

      // Check the version!
      compiledStream->read(&version);
      if(version != Con::DSOVersion)
      {
         Con::warnf("exec: Found an old DSO (%s, ver %d < %d), ignoring.", nameBuffer, version, Con::DSOVersion);
         ResourceManager->closeStream(compiledStream);
         compiledStream = NULL;
      }
   }

The fix again is simple...

// If we had a DSO, let's check to see if we should be reading from it.
   if(compiled && rCom && (!rScr || Platform::compareFileTimes(comModifyTime, scrModifyTime) >= 0))
   {
      compiledStream = ResourceManager->openStream(nameBuffer);
      if ( compiledStream )
      {
         // Check the version!
         compiledStream->read(&version);
         if(version != Con::DSOVersion)
         {
            Con::warnf("exec: Found an old DSO (%s, ver %d < %d), ignoring.", nameBuffer, version, Con::DSOVersion);
            ResourceManager->closeStream(compiledStream);
            compiledStream = NULL;
         }
      }
   }

And this seems to fix it. I'm a little worried as this seems like such a basic bug to have in such a well tested part of Torque, but TGB does go out of it's way to delete a DSO at runtime... something that isn't commonly done.

Someone should also check if this bug is present in TGE and TSE... i figure this fix should be applied there as well.