Cause of data corruption in C++ vector?

Question

I'm trying to write a basic MP3 player to try and get into some Windows stuff for C++, and things were going well until now. I tried writing a queuing class to play more songs after the current one ended, every time a new song is added to the queue, all the other data in the queue seems to get corrupted, which obviously causes issues. The corruption seems to be occurring between lines 202 and 212 in the handleOpen() function in my main file. I've tried changing variable scope and messing with the insertion code, but none of that fixed the issue. I usually write in Java, so C++ is a bit hard sometimes for me.

Main.cpp

#include <Windows.h>
#include <tchar.h>
#include <ShObjIdl.h>
#include "Player.h"
#include "Song.h"
#include "Queue.h"

#define BUTTON1 1
#define BUTTON2 2
#define NEXTBUTTON 3
#define PREVBUTTON 4

//I think these are just function prototypes
int CALLBACK WinMain(_In_ HINSTANCE hInstance, _In_opt_ HINSTANCE hPrevInstance, _In_ LPSTR lpCmdLine, _In_ int nCmdShow);

LRESULT CALLBACK WndProc(_In_ HWND hWnd, _In_ UINT message, _In_ WPARAM wParam, _In_ LPARAM lParam);

HRESULT BasicFileOpen(PWSTR* ptr_to_path);

void handleOpen(void);

//some static variables
static TCHAR szWindowClass[] = _T("Desktop App");
static TCHAR szTitle[] = _T("MP3 Player");

HWND h_window;
HWND h_button1;
HWND h_button2;
HWND h_nextbutton;
HWND h_prevbutton;

Player player = NULL;
Queue queue = Queue();
PWSTR path;
Song s;
wchar_t* ext;

int x = GetSystemMetrics(SM_CXSCREEN);
int y = GetSystemMetrics(SM_CYSCREEN);

//function definitions
int CALLBACK WinMain(_In_ HINSTANCE hInstance, _In_opt_ HINSTANCE hPrevInstance, _In_ LPSTR lpCmdLine, _In_ int nCmdShow) {
    //struct for a window
    WNDCLASSEX window;

    //initialize struct values
    window.cbSize = sizeof(WNDCLASSEX);
    //to combine multiple flags, OR them together
    window.style = CS_HREDRAW | CS_VREDRAW;
    //I think this defines the process function for window events
    window.lpfnWndProc = WndProc;
    window.cbClsExtra = 0;
    window.cbWndExtra = 0;
    window.hInstance = hInstance;
    window.hIcon = LoadIcon(hInstance, IDI_APPLICATION);
    window.hCursor = LoadCursor(NULL, IDC_ARROW);
    window.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
    window.lpszMenuName = NULL;
    window.lpszClassName = szWindowClass;
    window.hIconSm = LoadIcon(window.hInstance, IDI_APPLICATION);

    if (!RegisterClassEx(&window)) {
        MessageBox(NULL, _T("Call to RegisterClassEx failed!"), szTitle, NULL);

        return 1;
    }

    h_window = CreateWindow(szWindowClass, szTitle, WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, GetSystemMetrics(SM_CXSCREEN), GetSystemMetrics(SM_CYSCREEN), NULL, NULL, hInstance, NULL);

    //check if the window was create successfully
    if (!h_window) {
        MessageBox(NULL, _T("Call to CreateWindow Failed"), szTitle, NULL);

        return 1;
    }

    h_button1 = CreateWindow(L"BUTTON", L"Open", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 100, y - 200, 200, 50, h_window, (HMENU)BUTTON1, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
    h_button2 = CreateWindow(L"BUTTON", L"\u25B6", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 100, y - 140, 200, 50, h_window, (HMENU)BUTTON2, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
    h_nextbutton = CreateWindow(L"BUTTON", L"\u23E9", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) + 110, y - 140, 200, 50, h_window, (HMENU)NEXTBUTTON, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
    h_prevbutton = CreateWindow(L"BUTTON", L"\u23EA", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 310, y - 140, 200, 50, h_window, (HMENU)PREVBUTTON, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
    //tell the OS to show the window
    ShowWindow(h_window, nCmdShow);
    UpdateWindow(h_window);

    //send any messages to the process function
    MSG msg;
    while (GetMessage(&msg, NULL, 0, 0)) {
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }

    return (int)msg.wParam;
}

LRESULT CALLBACK WndProc(_In_ HWND hWnd, _In_ UINT message, _In_ WPARAM wParam, _In_ LPARAM lParam) {
    PAINTSTRUCT paint_struct;
    HDC hdc;
    //TCHAR greeting[] = _T("Hello Windows Desktop!");

    switch (message) {
        //handle button presses based on IDs
    case WM_COMMAND:
        switch (LOWORD(wParam)) {
        case BUTTON1:
            //PWSTR path;
            handleOpen();
            break;
        case BUTTON2:
            //MessageBox(NULL, _T("BUTTON2 Clicked"), szTitle, NULL);
            if (player.paused && player.playing) {
                h_button2 = CreateWindow(L"BUTTON", L"\u23F8", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 100, y - 140, 200, 50, h_window, (HMENU)BUTTON2, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
                player.play(h_window);
                //player.paused = false;
            }
            else if (!player.paused && player.playing) {
                h_button2 = CreateWindow(L"BUTTON", L"\u25B6", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 100, y - 140, 200, 50, h_window, (HMENU)BUTTON2, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
                player.pause();
                //player.paused = true;
            }
            else {
                player = Player(queue.getCur().ext);
                player.open(queue.getCur().path);
                player.play(h_window);
                h_button2 = CreateWindow(L"BUTTON", L"\u25B6", WS_TABSTOP | WS_VISIBLE | WS_CHILD | BS_PUSHBUTTON, (x / 2) - 100, y - 140, 200, 50, h_window, (HMENU)BUTTON2, (HINSTANCE)GetWindowLong(h_window, GWL_HINSTANCE), NULL);
            }
            break;
        default:
            break;
        }
        break;
    case WM_PAINT:
        hdc = BeginPaint(hWnd, &paint_struct);

        //update window stuff in here
        //TextOut(hdc, 5, 5, greeting, _tcslen(greeting));

        EndPaint(hWnd, &paint_struct);
        break;

    case MM_MCINOTIFY:
        player.close();
        queue.advance();
        //if (lstrcmpW(queue.getCur().path, L"") != 0) {
        player = Player(queue.getCur().ext);
        player.open(queue.getCur().path);
        player.play(h_window);
        //}
        break;

    case WM_DESTROY:
        PostQuitMessage(0);
        break;

    default:
        return DefWindowProc(hWnd, message, wParam, lParam);
        break;
    }
}

HRESULT BasicFileOpen(PWSTR* ptr_to_path) {
    HRESULT hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
    if (SUCCEEDED(hr)) {
        IFileOpenDialog* p_file_open;

        hr = CoCreateInstance(CLSID_FileOpenDialog, NULL, CLSCTX_ALL, IID_IFileOpenDialog, reinterpret_cast<void**>(&p_file_open));
        if (SUCCEEDED(hr)) {
            hr = p_file_open->Show(NULL);

            if (SUCCEEDED(hr)) {
                IShellItem* p_item;
                hr = p_file_open->GetResult(&p_item);
                if (SUCCEEDED(hr)) {
                    PWSTR file_path;
                    hr = p_item->GetDisplayName(SIGDN_FILESYSPATH, &file_path);
                    *ptr_to_path = file_path;
                    //if (SUCCEEDED(hr)) {
                    //  MessageBox(NULL, file_path, L"File Path", MB_OK);
                    //  CoTaskMemFree(file_path);
                    //}
                    CoTaskMemFree(file_path);
                    p_item->Release();
                }
            }
            p_file_open->Release();
        }
        CoUninitialize();
    }
    return hr;
}

void handleOpen(void) {
    if (SUCCEEDED(BasicFileOpen(&path))) {
        //check if the selected file is a supported audio format
        ext = wcsrchr(path, L'.');
        ext = ext ? ext + 1 : path;
        //MessageBox(NULL, ext, L"Extension", MB_OK);
        if (!lstrcmpW(ext, L"mp3")) {
            //Song s;
            s.path = path;
            s.ext = ext;
            queue.add(s);
            MessageBox(NULL, L"Added MP3 to Queue", L"", MB_OK);
        }
        else {
            MessageBox(NULL, path, L"Invalid or unsupported file extension", MB_OK);
        }
    }
}

Queue.cpp

#include <Windows.h>
#include "Song.h"
#include "Player.h"
#include "Queue.h"
#include <vector>
#include <iterator>

Queue::Queue() {
    q.reserve(32);
    it = q.begin();
    cur = 0;
}

//inserts a song into the queue at the start
void Queue::add(Song s) {
    //q.insert(q.begin(), 1, s);
    q.push_back(s);
}

//removes all instances of a specified song from the queue
//returns -1 if the song was not found, 0 if successfully removed
int Queue::remove(Song s) {
    bool found = false;
    for (std::vector<Song>::iterator i = q.begin(); i != it; ++i) {
        if (!lstrcmpW(s.path, (*i).path)) {
            q.erase(i);
            found = true;
        }
    }
    return found ? 0 : -1;
}

//sets the current song to the specified Song object
//returns -1 if the song was not found in the queue, 0 if successful
int Queue::seek(Song s) {
    for (unsigned int i = 0; i < q.size(); i++) {
        if (!lstrcmpW(s.path, q[i].path)) {
            cur = i;
            return 0;
        }
    }
    return -1;
}

//sets the current song to the specified indice
//same return values as the previous function
int Queue::seek(int i) {
    if (i < q.size() - 1) {
        cur = i;
        return 0;
    }
    return -1;
}

//returns the Song current song in the queue
Song Queue::getCur(void) {
    return q[cur];
}

//returns the previous song in the queue, or an empty one if none exists
Song Queue::getPrev(void) {
    if (cur - 1 > 0) {
        return q[cur - 1];
    }
    Song s{};
    return s;
}

//returns the next song in the queue, or an empty one if none exists
Song Queue::getNext(void) {
    if (cur + 1 < q.size()) {
        return q[cur + 1];
    }
    Song s{};
    return s;
}

void Queue::advance(void) {
    cur++;
}

Player.cpp

    #include <Windows.h>
    #include "Player.h"

    #pragma comment(lib, "WinMM.lib")



Player::Player(wchar_t* ext) {
        this->ext = ext;
        playing = false;
        paused = true;
        started = false;
    }

    MCIERROR Player::open(wchar_t* filepath) {
        if (playing) {
            mciSendString(L"close mp3", NULL, 0, NULL);
        }
        wchar_t command[1024] = L"open \"";
        if (sizeof(filepath) < 1000) {
            lstrcatW(command, filepath);
            lstrcatW(command, (wchar_t*)L"\" type mpegvideo alias mp3");
        }
        playing = true;
        return mciSendStringW(command, NULL, 0, NULL);
    }

    MCIERROR Player::play(HWND handle) {
        paused = false;
        if (started) {
            return mciSendString(L"resume mp3 notify", NULL, 0, handle);
        } else {
            started = true;
            return mciSendString(L"play mp3 notify", NULL, 0, handle);
        }
    }

    MCIERROR Player::restart(void) {
        return mciSendString(L"play mp3 from 0", NULL, 0, NULL);
    }

    MCIERROR Player::pause(void) {
        paused = true;
        return mciSendString(L"pause mp3", NULL, 0, NULL);
    }

    MCIERROR Player::close(void) {
        playing = false;
        paused = true;
        started = false;
        return mciSendString(L"close mp3", NULL, 0, NULL);
    }

The queue is basically just a vector with some added functionality, so I'm a bit confused where I am going wrong. Any help is greatly appreciated, thank you!

Answer 1

One issue is you're invalidating the q iterator in the Queue class in the remove function. You're removing an element from the vector while iterating over it.

To iterate over a vector and remove an element from it do the following:

for ( auto it = std::begin( q ); it != std::end( q ); )
{
     if ( condition )
         it = q.erase( it );
     else
         ++it;
}

Answer 2

Why not return success straight away when remove() finds the song, that way the iterator is not confused.

int Queue::remove(Song s) {
    for (std::vector<Song>::iterator i = q.begin(); i != it; ++i) {
        if (!lstrcmpW(s.path, (*i).path)) {
            q.erase(i);
            return 0;
        }
    }
    return -1;
}