Skip to content

Commit

Permalink
Bug 1828192 [Linux] Implement and use ManageChildProcess() to run and…
Browse files Browse the repository at this point in the history
… get data from child test process in GfxInfo r=emilio

- Implement ManageChildProcess() to get data from child test process and process exit status. We use glib I/O routines there instead of
  our custom implementation.
- Update GfxInfo::GetData() and GfxInfo::GetDataVAAPI() to use it.

Differential Revision: https://phabricator.services.mozilla.com/D175808
  • Loading branch information
stransky committed Apr 20, 2023
1 parent 1971088 commit eb7743a
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 159 deletions.
249 changes: 90 additions & 159 deletions widget/gtk/GfxInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@

#define VAAPI_PROBE_BINARY u"vaapitest"_ns

#define EXIT_STATUS_BUFFER_TOO_SMALL 2
#ifdef DEBUG
bool fire_glxtest_process();
#endif
Expand Down Expand Up @@ -104,96 +103,96 @@ void GfxInfo::AddCrashReportAnnotations() {
}
}

void GfxInfo::GetData() {
GfxInfoBase::GetData();

// to understand this function, see bug 639842. We retrieve the OpenGL driver
// information in a separate process to protect against bad drivers.

// if glxtest_pipe == -1, that means that we already read the information
if (glxtest_pipe == -1) {
return;
}
static bool MakeFdNonBlocking(int fd) {
return fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK) != -1;
}

const TimeStamp deadline =
TimeStamp::Now() + TimeDuration::FromMilliseconds(GFX_TEST_TIMEOUT);
static bool ManageChildProcess(int* aPID, int* aPipe, int aTimeout,
char** aData) {
GIOChannel* channel = nullptr;
*aData = nullptr;

enum { buf_size = 2048 };
char buf[buf_size];
ssize_t bytesread = 0;
auto free = mozilla::MakeScopeExit([&] {
if (channel) {
g_io_channel_unref(channel);
}
if (*aPipe >= 0) {
close(*aPipe);
*aPipe = -1;
}
if (*aPID) {
int status;
waitpid(*aPID, &status, WNOHANG);
*aPID = 0;
}
});

struct pollfd pfd {};
pfd.fd = glxtest_pipe;
pfd.fd = *aPipe;
pfd.events = POLLIN;
auto ret = poll(&pfd, 1, GFX_TEST_TIMEOUT);
auto ret = poll(&pfd, 1, aTimeout);
if (ret <= 0) {
gfxCriticalNote << "glxtest: failed to read data from glxtest, we may "
"fallback to software rendering\n";
} else {
// -1 because we'll append a zero
bytesread = read(glxtest_pipe, &buf, buf_size - 1);
gfxCriticalNote << "glxtest: poll failed: " << strerror(errno) << "\n";
return false;
}

channel = g_io_channel_unix_new(*aPipe);
MakeFdNonBlocking(*aPipe);

GUniquePtr<GError> error;
gsize length;
do {
error = nullptr;
ret = g_io_channel_read_to_end(channel, aData, &length,
getter_Transfers(error));
} while (ret == G_IO_STATUS_AGAIN);
if (error) {
gfxCriticalNote << "glxtest: failed to read data from child process: "
<< error->message << "\n";
return false;
}
close(glxtest_pipe);
glxtest_pipe = -1;

// bytesread < 0 would mean that the above read() call failed.
// This should never happen. If it did, the outcome would be to blocklist
// anyway.
if (bytesread < 0) {
bytesread = 0;
} else if (bytesread == buf_size - 1) {
gfxCriticalNote << "glxtest: read from pipe exceeded buffer size";
if (!length) {
gfxCriticalNote
<< "glxtest: failed to read data from child process, length = 0\n";
return false;
}

// let buf be a zero-terminated string
buf[bytesread] = 0;

// Wait for the glxtest process to finish. This serves 2 purposes:
// * avoid having a zombie glxtest process laying around
// * get the glxtest process status info.
int glxtest_status = 0;
bool wait_for_glxtest_process = true;
bool waiting_for_glxtest_process_failed = false;
int waitpid_errno = 0;
while (wait_for_glxtest_process) {
wait_for_glxtest_process = false;
if (waitpid(glxtest_pid, &glxtest_status, WNOHANG) == -1) {
waitpid_errno = errno;
if (waitpid_errno == EAGAIN || waitpid_errno == EINTR) {
wait_for_glxtest_process = true;
} else {
// Bug 718629
// ECHILD happens when the glxtest process got reaped after a
// PR_CreateProcess as per bug 227246. This shouldn't matter, as we
// still seem to get the data from the pipe, and if we didn't, the
// outcome would be to blocklist anyway.
waiting_for_glxtest_process_failed = (waitpid_errno != ECHILD);
}
}
if (wait_for_glxtest_process) {
if (TimeStamp::Now() > deadline) {
gfxCriticalNote << "glxtest: glxtest process hangs\n";
waiting_for_glxtest_process_failed = true;
break;
}
// Wait 100ms to another waitpid() check.
usleep(100000);
}
int status = 0;
int pid = *aPID;
*aPID = 0;
ret = waitpid(pid, &status, WNOHANG);
if (ret < 0) {
gfxCriticalNote << "glxtest: waitpid failed: " << strerror(errno) << "\n";
return false;
}

int exit_code = EXIT_FAILURE;
bool exited_with_error_code = false;
if (!waiting_for_glxtest_process_failed && WIFEXITED(glxtest_status)) {
exit_code = WEXITSTATUS(glxtest_status);
exited_with_error_code = exit_code != EXIT_SUCCESS;
if (WEXITSTATUS(status) != EXIT_SUCCESS) {
gfxCriticalNote << "glxtest: waitpid failed, status " << WEXITSTATUS(status)
<< "\n";
return false;
}

bool received_signal =
!waiting_for_glxtest_process_failed && WIFSIGNALED(glxtest_status);
return true;
}

bool error = waiting_for_glxtest_process_failed || exited_with_error_code ||
received_signal;
bool errorLog = false;
// to understand this function, see bug 639842. We retrieve the OpenGL driver
// information in a separate process to protect against bad drivers.
void GfxInfo::GetData() {
if (mInitialized) {
return;
}
mInitialized = true;

GfxInfoBase::GetData();

char* glxData = nullptr;
auto free = mozilla::MakeScopeExit([&] { g_free((void*)glxData); });

bool error = !ManageChildProcess(&glxtest_pid, &glxtest_pipe,
GFX_TEST_TIMEOUT, &glxData);
if (error) {
gfxCriticalNote << "glxtest: ManageChildProcess failed\n";
}

nsCString glVendor;
nsCString glRenderer;
Expand All @@ -218,8 +217,10 @@ void GfxInfo::GetData() {

nsCString* stringToFill = nullptr;
bool logString = false;
bool errorLog = false;

char* bufptr = glxData;

char* bufptr = buf;
while (true) {
char* line = NS_strtok("\n", &bufptr);
if (!line) break;
Expand Down Expand Up @@ -435,14 +436,14 @@ void GfxInfo::GetData() {
// If we still don't have a vendor ID, we can try the PCI vendor list.
if (mVendorId.IsEmpty()) {
if (pciVendors.IsEmpty()) {
gfxCriticalNote << "No GPUs detected via PCI";
gfxCriticalNote << "No GPUs detected via PCI\n";
} else {
for (size_t i = 0; i < pciVendors.Length(); ++i) {
if (mVendorId.IsEmpty()) {
mVendorId = pciVendors[i];
} else if (mVendorId != pciVendors[i]) {
gfxCriticalNote << "More than 1 GPU vendor detected via PCI, cannot "
"deduce vendor";
"deduce vendor\n";
mVendorId.Truncate();
break;
}
Expand All @@ -459,7 +460,7 @@ void GfxInfo::GetData() {
mDeviceId = pciDevices[i];
} else if (mDeviceId != pciDevices[i]) {
gfxCriticalNote << "More than 1 GPU from same vendor detected via "
"PCI, cannot deduce device";
"PCI, cannot deduce device\n";
mDeviceId.Truncate();
break;
}
Expand All @@ -471,7 +472,7 @@ void GfxInfo::GetData() {
if (!mVendorId.IsEmpty()) {
if (pciLen > 2) {
gfxCriticalNote
<< "More than 2 GPUs detected via PCI, secondary GPU is arbitrary";
<< "More than 2 GPUs detected via PCI, secondary GPU is arbitrary\n";
}
for (size_t i = 0; i < pciLen; ++i) {
if (!mVendorId.Equals(pciVendors[i]) ||
Expand All @@ -487,7 +488,7 @@ void GfxInfo::GetData() {
if (mVendorId.IsEmpty()) {
for (size_t i = 0; i < pciLen; ++i) {
gfxCriticalNote << "PCI candidate " << pciVendors[i].get() << "/"
<< pciDevices[i].get();
<< pciDevices[i].get() << "\n";
}
}

Expand Down Expand Up @@ -529,28 +530,6 @@ void GfxInfo::GetData() {
mGlxTestError = true;
}

if (error) {
nsAutoCString msg("glxtest: process failed");
if (waiting_for_glxtest_process_failed) {
msg.AppendPrintf(" (waitpid failed with errno=%d for pid %d)",
waitpid_errno, glxtest_pid);
}

if (exited_with_error_code) {
if (exit_code == EXIT_STATUS_BUFFER_TOO_SMALL) {
msg.AppendLiteral(" (buffer too small)");
} else {
msg.AppendPrintf(" (exited with status %d)",
WEXITSTATUS(glxtest_status));
}
}
if (received_signal) {
msg.AppendPrintf(" (received signal %d)", WTERMSIG(glxtest_status));
}

gfxCriticalNote << msg.get();
}

AddCrashReportAnnotations();
}

Expand Down Expand Up @@ -592,73 +571,25 @@ static int fire_vaapi_process(const char* aRenderDevicePath, int* aOutPipe) {
return pid;
}

static bool MakeFdNonBlocking(int fd) {
return fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK) != -1;
}

void GfxInfo::GetDataVAAPI() {
if (mIsVAAPISupported.isSome()) {
return;
}
mIsVAAPISupported = Some(false);

int vaapiPipe = -1;
int vaapiPID = 0;
gsize vaapiDataLen;
char* vaapiData = nullptr;
GIOChannel* channel = nullptr;

auto free = mozilla::MakeScopeExit([&] {
if (channel) {
g_io_channel_unref(channel);
}
if (vaapiPipe >= 0) {
close(vaapiPipe);
}
if (vaapiData) {
g_free((void*)vaapiData);
}
if (vaapiPID) {
int status;
waitpid(vaapiPID, &status, WNOHANG);
}
});
auto free = mozilla::MakeScopeExit([&] { g_free((void*)vaapiData); });

int vaapiPipe = -1;
int vaapiPID = 0;
vaapiPID = fire_vaapi_process(mDrmRenderDevice.get(), &vaapiPipe);
if (!vaapiPID) {
return;
}

struct pollfd pfd {};
pfd.fd = vaapiPipe;
pfd.events = POLLIN;
auto ret = poll(&pfd, 1, VAAPI_TEST_TIMEOUT);
if (ret <= 0) {
return;
}

channel = g_io_channel_unix_new(vaapiPipe);
MakeFdNonBlocking(vaapiPID);

GUniquePtr<GError> error;
do {
error = nullptr;
ret = g_io_channel_read_to_end(channel, &vaapiData, &vaapiDataLen,
getter_Transfers(error));
} while (ret == G_IO_STATUS_AGAIN);

if (error) {
return;
}

int vaapi_exit_code = EXIT_FAILURE;
int vaapi_status = 0;
if (waitpid(vaapiPID, &vaapi_status, WNOHANG) < 0) {
vaapiPID = 0;
return;
}
vaapi_exit_code = WEXITSTATUS(vaapi_status);
if (vaapi_exit_code != EXIT_SUCCESS) {
if (!ManageChildProcess(&vaapiPID, &vaapiPipe, VAAPI_TEST_TIMEOUT,
&vaapiData)) {
gfxCriticalNote << "vaapitest: ManageChildProcess failed\n";
return;
}

Expand Down
1 change: 1 addition & 0 deletions widget/gtk/GfxInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class GfxInfo final : public GfxInfoBase {
const nsAString& aDriverVendor) override;

private:
bool mInitialized = false;
nsCString mVendorId;
nsCString mDeviceId;
nsCString mDriverVendor;
Expand Down

0 comments on commit eb7743a

Please sign in to comment.