smasherprog / screen_capture_lite

cross platform screen/window capturing library
MIT License
616 stars 156 forks source link

Deadlocks in the code #127

Closed TopoIogist closed 2 years ago

TopoIogist commented 2 years ago

I am encountering deadlocks in my program which I could trace back to screen capture lite..

Changing the "Screen_Capture_Example.cpp" example slightly to this:

int main() {
    for(int i = 0; i < 100; ++i) {
        std::cout << "Iteration " << i << std::endl;
        createwindowgrabber();
        std::this_thread::sleep_for(std::chrono::seconds(2));
    }
}

and in createwindowgrabber

std::string srchterm = "Calculator";
...
framgrabber->setFrameChangeInterval(std::chrono::milliseconds(500));
    framgrabber->setMouseChangeInterval(std::chrono::milliseconds(500));

To trigger the issue I am opening up and closing many calc.exe windows. At some point a race condition occurs and the iteration counter does not progress anymore...

Iteration 6
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
ADDING WINDOW  Height 771  Width  1026   Calculator
onNewFrame fps0
Exiting Thread due to expected error
Iteration 7
TopoIogist commented 2 years ago

Basically what happens is this I think: The shared pointer gets destroyed and sets the TerminateThreadEvent bool. Then simultaneously this "cleanup code" in ScreenCapture.cpp related to the exception just removes the TerminateThreadsEvent bool...

                        Thread_Data_->CommonData_.TerminateThreadsEvent = false;
                        // Clean up
                        std::this_thread::sleep_for(std::chrono::milliseconds(1000)); // sleep for 1 second since an error occcured
smasherprog commented 2 years ago

Yeah I see that.. good catch!

There does appear to be a race between the start and the destructor running. I just pushed a change to master. Let me know if that works

TopoIogist commented 2 years ago

Yes looks fixed with the update