Solved

CFile::Rename() - hangs app with big files. Is a thread the solution? If so, how?

Posted on 2004-08-11
36
1,009 Views
Last Modified: 2008-01-09
Using VC++ 6.0.

My experience thus far with multi-threading is minimal. My application basically takes BMP screen-captures, putting them in the users temp folder, and at incremental points and when asked, "saving" by renaming appropriately into the actual storage path (files are named with a standard convention, using incremental integer values to index the resulting files in a folder. This rename/move happens with the CFile::Rename() method.  For my bitmap captures, this works quickly and is not an issue.

I've now added AVI capture support to my app, and am using the existing data management architecture to handle this. I'm finding though that the CFile::Rename() function, when called with a file that is potentially several hundred MB in size, pretty much slows my app to a standstill until it's complete. The UI still accepts mouse-clicks, rollovers etc. still work, but functionality stops. My messages are queued up and so eventually the app snaps back alive and quickly shoots through all actions the user may have tried during this freeze.

This to me says Thread (I think.. though it's only the one CFile::Rename() call that would have to be in a thread.. or is the issue disc access??). Basically everything's in place and working as expected, I'm only trying to fix the problem with the app hanging up until the files are renamed.

Each export is an instance of CMovie, which contains the following method (among many)

BOOL CMovie::SaveFilename(LPCTSTR path, LPCTSTR label, CString &name)
{
      if (!m_bTempFile)
      {
            name = m_tmpName;
            return TRUE;
      }
      CString sOldName = GetFilename();

      // we need to rename
      CString sNewName;            
      CString sPath = path;
      if (sPath.Right(1) != _T("\\") && sPath.Right(1) != _T(":"))
            sPath += _T("\\");

      CString format = label;                  
      format += _T("_Movie%.3d.AVI");

      long lCounter = 0;
      while (++lCounter < 100000)
      {
            sNewName.Format(format, lCounter);
            TRY
            {
                  CFileStatus stat;
                  BOOL bCreate = FALSE;
                  if (!CFile::GetStatus(sPath+sNewName, stat))
                  {
                        // this is our new name...
                        CFile::Rename(sOldName, sPath+sNewName);
                        m_MovieID = sNewName.Left(sNewName.GetLength() - 4);
                        m_tmpName = sNewName;
                        name = sNewName;
                        m_bTempFile = FALSE;
                        m_sPath = sPath;
                        return TRUE;
                  }
            } CATCH (CFileException, pFE)
            {
                  // some problems... try another one
            } END_CATCH            
      }
      return FALSE;
}

that while loop simply acts to find the next incremental index value to tack onto the filename it's saving. Generally this will only go 5-10 times depending on how many captures the user performed. My question is, what code do I need to add to put the CFile::Rename() call into a worker thread?  OR - is a thread not the answer? I could do a bunch of re-work on the existing file management architecutre, to not use the users ~TEMP folder for intermediary saves of the AVI files, rather, placing them in the final storage location from the get-go, but that will require re-writing (or placeing elsewhere)  some of the fundamental stuff that defines and sets up the directory structures in the first place. Doable, but if a thread is appropriate, I'd rather do that.


Thanks!
-Paul
0
Comment
Question by:PMH4514
  • 16
  • 14
  • 3
  • +2
36 Comments
 
LVL 86

Expert Comment

by:jkr
ID: 11774156
Have you tried to use 'MoveFile()'  or 'MoveFileEx()' instead? This is way closer to the system...
0
 
LVL 14

Expert Comment

by:wayside
ID: 11774279
Are you renaming to a different volume? I believe in this case the file is actually copied. If your AVI is large (it will surely be much larger than a .bmp file), that may be why it is taking so long; a thread running at a lower priority would definitely help.

> Have you tried to use 'MoveFile()

CFile::Rename() just calls MoveFile() with no further processing. It doesn't seem like using MoveFile() directly would help. MoveFileEx()... the available flags don't seem that helpful.

0
 
LVL 22

Expert Comment

by:grg99
ID: 11774401
Watch your disk ights during the rename.  If they flicker for more than 0.66666 second, then it's probably copying the file instead of renaming it.   This happens when the file is renamed across physical or logical disks.    The solution is simple, just write your file to the destination disk instead of whereever.

0
 

Author Comment

by:PMH4514
ID: 11774497
yes, the files are being moved from the users ~TEMP folder on C: onto a physically seperate volume.
so how do I get that Rename call to run in a thread?
0
 
LVL 86

Accepted Solution

by:
jkr earned 125 total points
ID: 11775172
You could use

struct MoveParams {

TCHAR pszSrc [MAX_PATH];
TCHAR pszDst [MAX_PATH];
HANDLE hComplete;
BOOL bDeleteThis;
};

ULONG WINAPI FileMoveThread ( LPVOID pv) {

    MoveParams* pParam = (MoveParams*) pv;
    MoveFileEx ( pParam->pszSrc, pParam->pszDst, MOVEFILE_WRITE_THROUGH);

    if ( pParam->hComplete) SetEvent ( pParam->hComplete);
    if ( pParam->bDeleteThis) delete pParam;

void AsyncMoveFile ( LPCTSTR pszSrc, LPCTSTR pszDst) {

    DWORD dwTID;
    MoveParams* p = new MoveParams;

    _tcscpy ( p->pszSrc, pszSrc);
    _tcscpy ( p->pszDst, pszDst);
    p->bDeleteThis = TRUE;
    p->hComplete = NULL;

    CreateThread ( NULL, 0, FileMoveThread, (LPVOID) p, 0, &dwTID);
}
0
 
LVL 86

Expert Comment

by:jkr
ID: 11775197
Ooops, the thread func should be

ULONG WINAPI FileMoveThread ( LPVOID pv) {

   MoveParams* pParam = (MoveParams*) pv;
   BOOL b = MoveFileEx ( pParam->pszSrc, pParam->pszDst, MOVEFILE_WRITE_THROUGH);

   if ( pParam->hComplete) SetEvent ( pParam->hComplete);
   if ( pParam->bDeleteThis) delete pParam;

   if ( !b) return GetLastError ();

   return 0;
}

0
 
LVL 22

Expert Comment

by:grg99
ID: 11776085
Is there some overwhelmingly good reason you're writing to ~TEMP?  Why not write to the correct target disk and avoid all the copying folderol?
0
 

Author Comment

by:PMH4514
ID: 11776285
>>Is there some overwhelmingly good reason you're writing to ~TEMP?

because the existing file management architecture is already in place (put in place prior to my coming onboard, I'm not sure I agree with the decision, but there's a lot of code tying this all together that I'd rather not re-write during this time frame) and it handles checking for existence of, and setting up the target directories based on other system conditions  Data is handled during runtime and may or may not end up in the target location. etc. Would I prefer to not deal with all this copying? of course, but I have a deadline and a short QA cycle to boot, dont want to force a full regression test.

JKR- thanks for that code.. going off to check it out now.
0
 

Author Comment

by:PMH4514
ID: 11776423
JKR - I'll note that this code will go in my CMovie class. That said, I assume I need to add those interfaces to my header file;

private:
    ULONG WINAPI FileMoveThread ( LPVOID pv);
    void AsyncMoveFile ( LPCTSTR pszSrc, LPCTSTR pszDst);

right? I did that, but then it won't compile, I get this error:
error LNK2001: unresolved external symbol "private: void __thiscall CMovie::AsyncMoveFile(char const *,char const *)" (?AsyncMoveFile@CMovie@@AAEXPBD0@Z)

so I change in my code file to:
void CMovie::AsyncMoveFile ( LPCTSTR pszSrc, LPCTSTR pszDst) {

but if I try to compile that, I get:
error C2664: 'CreateThread' : cannot convert parameter 3 from 'unsigned long (void *)' to 'unsigned long (__stdcall *)(void *)'
        None of the functions with this name in scope match the target type

0
 
LVL 86

Expert Comment

by:jkr
ID: 11776468
The thread function cannot be a 'regular' class member, since these are passed a hidden 'this' pointer as the 1st parameter. I'd suggest you make that a global function or alternatively declare that as 'static', e.g.

private:
   static ULONG WINAPI FileMoveThread ( LPVOID pv);
   void AsyncMoveFile ( LPCTSTR pszSrc, LPCTSTR pszDst);

0
 

Author Comment

by:PMH4514
ID: 11776525
oh I see.  Do I have to worry about killing this thread off or will it kill itself?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 11776541
Or

// global variables
CString  g_path;
CString  g_label;
CString  g_name;

// ThreadProc
DWORD WINAPI RenameAvi(
  LPVOID lpParameter   // thread data
)
{
      CMovie* pMovie = (ThreadData*)lpParameter;
      pMovie->SaveFilename(g_path, g_label, g_name);    
      pMovie->SetThreadStatus(FALSE);
}

BOOL CMovie::SaveFilenameThreaded(LPCTSTR path, LPCTSTR label, CString &name)
{
     if (!SetThreadStatus(TRUE))
     {
          AfxMessageBox("Couldn't save another file while saving is still in progress");
          return FALSE;
     }
     HANDLE hThread = CreateThread(NULL, 0, RenameAvi, this, 0, NULL);
     if (!hThread)
           return FALSE;
     SetThreadPriority(hThread, THREAD_PRIORITY_LOWEST);
     return TRUE;
}      

BOOL CMovie::SetThreadStatus(BOOL bIsRunning)
{
     // m_bThreadIsRunning is a private BOOL member initialised to FALSE
     
     if (bIsRunning && m_bThreadIsRunning)
           return FALSE;// could not run two threads
     
    m_bThreadIsRunning = bIsRunning;
}

Note, jkr's solution is able to run more than one thread.

Note, if you stored path, label und name as members as you did it with filename, you don't need global variables in my solution.

Regards, Alex
 

 
0
 
LVL 86

Expert Comment

by:jkr
ID: 11776560
>>Do I have to worry about killing this thread off or will it kill itself?

No, it will just terminate when moving/renaming the file is done. If you provide an event it the parameter struct, it'll even signal the completion by setting this event.
0
 

Author Comment

by:PMH4514
ID: 11776668
thanks Alex - I do need multiple threads though, several movies may be stored in TEMP before one or more of them is moved over. Nice code though, thanks.

JKR - I've got your code integrated. it compiles and runs but MoveFileEx tells me "the system cannot move the file to a different disk drive".. If I add the MOVEFILE_COPY_ALLOWED flag it works!

  BOOL b = MoveFileEx ( pParam->pszSrc, pParam->pszDst, MOVEFILE_WRITE_THROUGH | MOVEFILE_COPY_ALLOWED);

I did notice though that even though it works, my variable watcher shows "The system could not find the environment option that was entered" after that line is executed.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11776703
>>my variable watcher shows "The system could not find the environment option that was entered" after that line is
>>executed

Regarding which variable?
0
 

Author Comment

by:PMH4514
ID: 11776802
oh I keep @err,hr in my variable watcher.. that's where it was displayed.

uh oh, I'm noticing now that my files are copied, but not deleted from the TEMP directory..  what am I doing wrong?
0
 

Author Comment

by:PMH4514
ID: 11776873
hmm.. also - while my AVI files in the TEMP directory (still undeleted, see my last comment) play properly, if I try to open the copied version with windows media player it says it's encountered an unknown error, and shuts down. both files show the same size value.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11776877
Are the files marked as 'read only'?
0
Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 
LVL 86

Expert Comment

by:jkr
ID: 11776917
You could try to

ULONG WINAPI FileMoveThread ( LPVOID pv) {

  MoveParams* pParam = (MoveParams*) pv;
  BOOL b;

  b = CopyFile ( pParam->pszSrc, pParam->pszDst, FALSE);

   if ( !b) {

    TRACE1 ( "'CopyFile()' failed, error == %d", GetLastError());

    return -1;
  }

  b = DeleteFile ( pParam->pszSrc);

   if ( !b) {

    TRACE1 ( "'DeleteFile()' failed, error == %d", GetLastError());

    return -2;
  }

  if ( pParam->hComplete) SetEvent ( pParam->hComplete);
  if ( pParam->bDeleteThis) delete pParam;

  if ( !b) return GetLastError ();

  return 0;
}
0
 

Author Comment

by:PMH4514
ID: 11777122
not marked Read Only.

the file gets removed if I step through the code, but not if it is allowed to run on its own. The copy always works, but again, the copied version of the AVI file won't play.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11777184
That's weird, *very* weird and I can assure you that "copied version of the AVI file won't play" issue has nothing to do with multithreading. BTW, just to make that one sure: The files are all closed, aren't they?
0
 

Author Comment

by:PMH4514
ID: 11777252
>>I can assure you that "copied version of the AVI file won't play" issue has nothing to do with multithreading.
yeah, I wouldn't expect it did.

>>The files are all closed, aren't they?
yes. During my dev efforts I've had plenty of opportunity to try to open them with Windows Media Player before they were closed, it wasn't pretty :-) These files in the TEMP Folder, I can dbl-click and view them all the way to the last frame and then it stops playing. The CopyFile() works, the destination file reports in Windows Explorer the exact same size as the file in TEMP, but it won't play. If I right click on the new file and go to Properties tab, windows explorer kinda chugs and chugs, then not responding, and then I have to click End Now.

I tried using your code to replace the CFile::Rename() call in my Image class (which handles exported Bitmaps in the same fashion). In that case, the TEMP bitmap files are all deleted, and the copied versions open in Paint and look correct.


0
 

Author Comment

by:PMH4514
ID: 11777364
actually, your code is working, even with the AVI files.  The bugs I'm seeing happen when I am trying to export single frames between 2 video exports.. which is totally unrelated to this thread. Your code worked wonderfully..

thanks!
Paul
0
 
LVL 86

Expert Comment

by:jkr
ID: 11777416
*phew* :o)
0
 

Author Comment

by:PMH4514
ID: 11835088
*phew* - I wish.. I got sidetracked, something else came up.. looking deeper into this, I am still having the problem, though it seems that sometimes it works as expected.. This is very strange.  I have two AVI files, one the source, one the destination created with CopyFile(). Both show the same filesize. the source plays fine, the new one does not play.

I downloaded this hex comparison tool: http://www.fairdell.com/hexcmp/

The files are identical until offset 025F0000, at which point the destination file is all NULL until the end of file.

it's almost as if the file is created but the thread is dying off before the copy is complete? It still seems (though not always true) that if I step through the code, it works. but not otherwise.

Also I'm noticing that when I close my app (running in debug mode) I see this:
-----------------------------------
he thread 0x134 has exited with code 0 (0x0).
Detected memory leaks!
Dumping objects ->
{16020} normal block at 0x020E8570, 528 bytes long.
 Data: <C:\DOCUME~1\PHEM> 43 3A 5C 44 4F 43 55 4D 45 7E 31 5C 50 48 45 4D
strcore.cpp(118) : {14755} normal block at 0x020E68D8, 59 bytes long.
 Data: <    .   .   E:\V> 01 00 00 00 2E 00 00 00 2E 00 00 00 45 3A 5C 56
{14753} normal block at 0x015BEF30, 8 bytes long.
 Data: <t ^  h  > 74 10 5E 00 E4 68 0E 02
Object dump complete.
-----------------------------------

that "<C:\DOCUME~1\PH>" is my temp path.  (C:\Documents and Settings\ph\Local Settings\Temp)

Any ideas? This is totally baffling me.



0
 
LVL 86

Expert Comment

by:jkr
ID: 11835121
Hmm, what's your actual code now?
0
 

Author Comment

by:PMH4514
ID: 11835205
hasn't changed (see below)
I tried a couple times, the offset at which the copied file is all NULL values is different each time.. (so is the length of the AVI file, but that is expected). Also, the files don't ever get deleted when this fails. When the copy is successful and the new file plays (which I can't duplicate, but it does happen once in a while) the source/temp file is deleted successfully. Even when it doesn't work, none of the TRACE messages below are output.

AsyncMoveFile(sOldName, sPath+sNewName);


global code:
void AsyncMoveFile ( LPCTSTR pszSrc, LPCTSTR pszDst) {

    DWORD dwTID;
    MoveParams* p = new MoveParams;

    _tcscpy ( p->pszSrc, pszSrc);
    _tcscpy ( p->pszDst, pszDst);
    p->bDeleteThis = TRUE;
    p->hComplete = NULL;

    CreateThread ( NULL, 0, FileMoveThread, (LPVOID) p, 0, &dwTID);
}

ULONG WINAPI FileMoveThread ( LPVOID pv) {

  MoveParams* pParam = (MoveParams*) pv;
  BOOL b;

  b = CopyFile ( pParam->pszSrc, pParam->pszDst, FALSE);

   if ( !b) {

    TRACE ( "'CopyFile()' failed, error == %d", GetLastError());

    return -1;
  }

  b = DeleteFile ( pParam->pszSrc);

   if ( !b) {

    TRACE ( "'DeleteFile()' failed, error == %d", GetLastError());

    return -2;
  }

  if ( pParam->hComplete) SetEvent ( pParam->hComplete);
  if ( pParam->bDeleteThis) delete pParam;

  if ( !b) return GetLastError ();

  return 0;
}
0
 
LVL 86

Expert Comment

by:jkr
ID: 11835259
Hm, so it could well be possible that either 'CopyFile()' or 'DeleteFile()' fails - check the debug output. The leak can be tackled using

ULONG WINAPI FileMoveThread ( LPVOID pv) {

  MoveParams* pParam = (MoveParams*) pv;
  BOOL b;

  b = CopyFile ( pParam->pszSrc, pParam->pszDst, FALSE);

   if ( !b) {

    TRACE ( "'CopyFile()' failed, error == %d", GetLastError());
    if ( pParam->bDeleteThis) delete pParam;

    return -1;
  }

  b = DeleteFile ( pParam->pszSrc);

   if ( !b) {

    TRACE ( "'DeleteFile()' failed, error == %d", GetLastError());
    if ( pParam->bDeleteThis) delete pParam;

    return -2;
  }

  if ( pParam->hComplete) SetEvent ( pParam->hComplete);
  if ( pParam->bDeleteThis) delete pParam;

  if ( !b) return GetLastError ();

  return 0;
}
0
 
LVL 14

Expert Comment

by:wayside
ID: 11835375
Is the destination drive slower than the source drive? Are both drives on the same computer?

I have a problem on my pc at home where if I try to copy lots of data from a fast drive to a slower, smaller drive the copy eventually dies and sometimes hangs the machine.

The fast drive is ata66, the slow is ata33; there are special drivers for the ata66 drive and there may be a bug in there somewhere.

One way around might be to implement your own "copy" functionality, ie

open source
open destination
while not eof on the source
    read some bytes from the source
    write them to the destination
    flush the destination buffer

This might get around any problems with buffer overruns or bus contention or driver issues or whatever.
0
 

Author Comment

by:PMH4514
ID: 11835616
Thanks jkr - I'll check that out. Wayside - both drives are on the same computer, both are 5400rpm.
0
 

Author Comment

by:PMH4514
ID: 11835696
is there anyway to ensure that this thread isn't killed before the copy is complete?
0
 
LVL 86

Expert Comment

by:jkr
ID: 11835714
It will not be killed before, unless you call 'TerminateThread()' explicitly...
0
 
LVL 14

Expert Comment

by:wayside
ID: 11835739
If you use the Windows explorer to copy the file, does it succeed?
0
 

Author Comment

by:PMH4514
ID: 11835924
>>It will not be killed before, unless you call 'TerminateThread()' explicitly...
I didn't think so.

>>If you use the Windows explorer to copy the file, does it succeed?
yes.




0
 

Author Comment

by:PMH4514
ID: 11871119
hmm.. still having this problem.. seems to work just fine the first time I run through it, any subsequent runs of the app (with full shutdown between) and the problem persists..I should probably start a new question since this one is closed.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11873487
Hm, so is the problem with 'DeleteFile()' than?
0

Featured Post

How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

Join & Write a Comment

This article will show you some of the more useful Standard Template Library (STL) algorithms through the use of working examples.  You will learn about how these algorithms fit into the STL architecture, how they work with STL containers, and why t…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.

746 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

16 Experts available now in Live!

Get 1:1 Help Now