PCSX2 hang on exit (Linux)...
#21
The Cancel() is waiting for the read() to complete. There was a FIXME there to make sure I would look at it again.

EDIT: The logic is slightly more broken than I thought it was.
Reply

Sponsored links

#22
On the winconsolepipe they have the following:
Code:
    // Cleanup Order Notes:
    //  * The redirection thread is most likely blocking on ReadFile(), so we can't Cancel yet, lest we deadlock --
    //    Closing the writepipe (either directly or through the fp/crt handles) issues an EOF to the thread,
    //    so it's safe to Cancel afterward.

    if( m_fp != NULL )
    {
        fclose( m_fp );
        m_fp = NULL;
    }

    m_Thread.Cancel();

Guess the same will apply to linux?
Reply
#23
Not really, closing the file descriptor when it's blocked in the middle of a syscall is undefined behaviour on Linux, which is why I didn't do that.
Reply
#24
Actually, the close might work, I was thinking about select() when I mentioned undefined behaviour.
Reply
#25
If you are open to suggestions the following is a bit crude but works well:

Code:
void LinuxPipeThread::ExecuteTaskInThread()
{
    char buffer[2049];
    static int halt_u16 = 0;
    ssize_t bytes_read;

    while( (bytes_read = read(m_read_fd, buffer, sizeof(buffer) - 1)) &&
           (halt_u16 == 0) ){
        TestCancel();

        if (bytes_read == -1) {
            if (errno == EINTR) {
                continue;
            }
            else {
                // Should never happen.
                Console.Error(wxString::Format(L"Redirect %s failed: read: %s",
                    m_color == Color_Red ? L"stderr" : L"stdout", strerror(errno)));
                break;
            }
        }

        buffer[bytes_read] = 0;
        
        if( buffer[0] = '#' )
        {
            halt_u16 = 1;
        }

        {
            ConsoleColorScope cs(m_color);
            Console.WriteRaw(fromUTF8(buffer));
        }

        TestCancel();
    }
}

Code:
void LinuxPipeRedirection::Cleanup() noexcept
{
    
    puts("a");
    
    // Restore stdout/stderr file descriptor - mostly useful if the thread
    // fails to start, but then you have bigger issues to worry about.
    if (m_pipe_fd[1] != -1) {
        puts("b");
        if (m_pipe_fd[1] == fileno(m_stdstream)) {
            puts("c");
            // FIXME: Use lock for better termination.
            // The redirect thread is most likely waiting at read(). Send a
            // newline so it returns and the thread begins to terminate.
            fflush(m_stdstream);
            puts("d");
            puts("e");
            //fputc('\n', m_stdstream);
            fputc('#', m_stdstream);
            
            puts("f");
            m_thread.Cancel();
            fflush(m_stdstream);
            puts("g");
            dup2(fileno(m_fp), fileno(m_stdstream));
            puts("h");
        } else {
            close(m_pipe_fd[1]);
        }
    }

    puts("i");
    
    if (m_fp != nullptr) {
        if (m_stdstream == stdout)
            Console_SetStdout(stdout);
        fclose(m_fp);
    }

    if (m_pipe_fd[0] != -1)
        close(m_pipe_fd[0]);
}


Explanation:

Because a method of closing the read thread was required I have added an extra condition with the "halt_u16 == 0" check.

This value can only be set to 1 if the '#' symbol is the first character in the buffer[].

In the cleanup thread I send fputc('#', m_stdstream); before trying to close the thread.

This makes the while condition false and the thread terminates quite happily Smile

Now I realize that '#' would not be suitable but there is no reason why a random string could not be parsed instead such as 'THREADCLOSE123456' (you get the idea).

I realize that this is really crude and not terribly elegant but it has the advantage of being predicatable behavior on any platform.
Reply
#26
Infact thinking about it. To save parsing a string etc. You could just create a function as part of the class that sets a variable true or false and then call that before closing the thread?
Reply
#27
Well, I've attached a patch which should work, though I still need to comment and tidy it up. Some of it just rearranges stuff so things are a bit simpler.

I must have been a bit out of it when I did the initial Linux console redirection stuff cause the close() does work.


Attached Files
.txt   fix_hang.txt (Size: 6,63 KB / Downloads: 168)
Reply
#28
Patch tested.

Works great. Thanks for looking into it.
Reply




Users browsing this thread: 1 Guest(s)