What's the best way to copy a file in a way that I can easily cancel the copy while it is in progress?

Question

I am using ReadFileEx to read some bytes from a file and using WriteFileEx to write some bytes to a device. This action will repeat till all file bytes are read and written to the device.

Reason I use Ex APIs is because by requesting overlapped IOs to the OS can keep UI thread responsive updating a progress bar while the read/write function is doing their tasks.

The process begins with a ReadFileEx and a MY_OVERLAPPED structure and a ReadCompletionRoutine will be passed in together. Once the read is done, the read completion routine will be called. Inside the routine, a WriteFileEx will be emitted and WriteCompletionRoutine will be called. Inside the write completion routine, another ReadFileEx will be emitted after the offset of the MY_OVERLAPPED structure is reset to next position. That is, two completions will call each other once a read or write is done.

Notice that the above process will only be executed if the calling thread is under alertable state. I use a while loop to keep the thread alertable by keep checking a global state variable is set to TRUE or not. The state variable, completed, will be set to TRUE inside ReadCompletionRoutine once all procedure is done.

FYI, MY_OVERLAPPED structure is a self-define structure that inherits OVERLAPPPED structure so that I can add 2 more information I need to it.

Now, my question is I would like to add a cancel function so that the user can cancel all the process that has been started. What I do is pretty simple. I set the completed variable to TRUE when a cancel button is pressed, so the while loop will break and alertable state will be stoped so the completion routines won't be executed. But, I don't know how to cancel the overlapped request that sent by the Read/WriteFileEx and their completion routines along with the MY_OVERLAPPED structure(see the //******* part in code). Now my code will crash once the cancel button is pressed. The cancel part is the one causing the crash. Please help, thank you.

//In MyClass.h========================================

struct MY_OVERLAPPED: OVERLAPPED {
    MyClass *event;
    unsigned long long count;
};


//In MyClass.cpp - main===============================

MY_OVERLAPPED overlap;
memset(&overlap, 0,sizeof(overlap));

//point to this class (MyClass), so all variables can later be accessed
overlap.event = this; 

//set read position
overlap.Offset = 0;
overlap.OffsetHigh = 0;
overlap.count = 0;

//start the first read io request, read 524288 bytes, which 524288 bytes will be written in ReadCompletionRoutine
ReadFileEx(overlap.event->hSource, overlap.event->data, 524288, &overlap, ReadCompletionRoutine);

while(completed != true) {
    updateProgress(overlap.count);
    SleepEx(0,TRUE);
}

//********
CancelIo(overlap.event.hSource);
CancelIo(overlap.event.hDevice);
//********


//In MyClass.cpp - CALLBACKs===============================  

void CALLBACK ReadCompletionRoutine(DWORD errorCode, DWORD bytestransfered, LPOVERLAPPED lpOverlapped)
{
    //type cast to MY_OVERLAPPED
    MY_OVERLAPPED *overlap = static_cast<MY_OVERLAPPED*>(lpOverlapped);

    //write 524288 bytes and continue to read next 524288 bytes in WriteCompletionRoutine
    WriteFileEx(overlap->event->hDevice, overlap->event->data, 524288, overlap, WriteCompletionRoutine);
}

void CALLBACK WriteCompletionRoutine(DWORD errorCode, DWORD bytestransfered, LPOVERLAPPED lpOverlapped)
{
    MY_OVERLAPPED *overlap = static_cast<MY_OVERLAPPED*>(lpOverlapped);
    if(overlap->count<fileSize/524288) {
    //set new offset to 524288*i, i = overlap->count for next block reading
    overlap->count = (overlap->count)+1;
    LARGE_INTEGER location;
    location.QuadPart = 524288*(overlap->count);
    overlap->Offset = location.LowPart;
    overlap->OffsetHigh = location.HighPart;

    ReadFileEx(overlap->event->hSource, overlap->event->data, 524288, overlap, ReadCompletionRoutine);
    }
    else {
    completed = TRUE;
    }
}

Note that I prefer not to use multi-thread programming. Other than that, any better way of accomplishing the same goals is appreciated. Please and feel free to provide detail code and explanations. Thanks.

Answer 1

I actually would use a background thread for this, because modern C++ makes this very easy. Much easier, certainly, than what you are trying to do at the moment. So please try to shed any preconceptions you might have that this is the wrong approach for you and please try to read this post in the spirit in which it is intended. Thanks.

First up, here's some very simple proof of concept code which you can compile and run for yourself to try it out. At first sight, this might look a bit 'so what?', but bear with me, I'll explain at the end:

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <thread>
#include <chrono>
#include <memory>
#include <atomic>

int usage ()
{
    std::cout << "Usage: copy_file infile outfile\n";
    return 255;
}

void copy_file (FILE *infile, FILE *outfile, std::atomic_bool *cancel)
{
    constexpr int bufsize = 32768;
    std::unique_ptr <char []> buf (new char [bufsize]);
    std::cout << "Copying: ";

    while (1)
    {
        if (*cancel)
        {
            std::cout << "\nCopy cancelled";
            break;
        }

        size_t bytes_read = fread (buf.get (), 1, bufsize, infile);
        if (bytes_read == 0)
        {
            // Check for error here, then break out of the loop
            break;
        }

        size_t bytes_written = fwrite (buf.get (), 1, bytes_read, outfile);
        // Again, check for error etc

        std::cout << ".";
    }

    std::cout << "\nCopy complete\n";
    // Now probably something like PostMessage here to alert your main loop hat the copy is complete
}

int main (int argc, char **argv)
{
    if (argc < 3) return usage ();

    FILE *infile = fopen (argv [1], "rb");
    if (infile == NULL)
    {
        std::cout << "Cannot open input file " << argv [1] << "\n";
        return 255;
    }

    FILE *outfile = fopen (argv [2], "wb");
    if (outfile == NULL)
    {
        std::cout << "Cannot open output file " << argv [2] << "\n";
        fclose (infile);
        return 255;
    }

    std::atomic_bool cancel = false;
    std::thread copy_thread = std::thread (copy_file, infile, outfile, &cancel);

    std::this_thread::sleep_for (std::chrono::milliseconds (200));
    cancel = true;
    copy_thread.join ();   // waits for thread to complete

    fclose (infile);
    fclose (outfile);      // + error check!
    std::cout << "Program exit\n";
}

And when I run this on my machine, I get something like:

background_copy_test bigfile outfile

Copying: 
.....................................................................................
..............
Copy cancelled
Copy complete
Program exit

So, what's noteworthy about this? Well, in no particular order:

• It's dead simple.
• It's standard C++. There are no Windows calls in there at all (and I did that deliberately, to try to make a point).
• It's foolproof.
• It 'just works'.

Now of course, you're not going to put your main thread to sleep while you're copying the file in real life. No no no. Instead, you're going to just kick the copy off via std::thread and then put up your 'Copying...' dialog with a Cancel button in it (presumably, this would be a modal dialog)

Then:

• If that button is pressed, just set cancel to true and the magic will then happen.
• Have copy_file send your 'copying' dialog a WM_APP+nnn message when it is done. It can also do that to have the dialog update its progress bar (I'm leaving all that stuff to you).
• Don't omit that call to join() before you destroy copy_thread or it goes out of scope!

What else? Well, to get your head around this properly, study a bit of modern C++. cppreference is a useful site, but you should really read a good book. Then you should be able to apply the lessons learned here to your particular use-case.

Edit: It occurs to me to say that you might do better to create your thread in the WM_INITDIALOG handler for your 'Copying' dialog. Then you can pass the dialog's HWND to copy_file so that it knows where to send those messages to. Just a thought.

And you have a fair bit of reading to do if you're going to profit from this post. But then again, you should. And this post is going to achieve precisely nothing, I fear. Shame.