Skip to content

Commit

Permalink
8324668: JDWP process management needs more efficient file descriptor…
Browse files Browse the repository at this point in the history
… handling

Reviewed-by: gziemski, dholmes, cjplummer
  • Loading branch information
jaikiran committed Feb 1, 2024
1 parent a2229b1 commit a663248
Showing 1 changed file with 109 additions and 15 deletions.
124 changes: 109 additions & 15 deletions src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -23,12 +23,16 @@
* questions.
*/

#include <dirent.h>
#include <errno.h>
#include <stdlib.h>
#include <sys/resource.h>
#include <unistd.h>
#include <string.h>
#include <ctype.h>
#include "sys.h"
#include "util.h"
#include "error_messages.h"

static char *skipWhitespace(char *p) {
while ((*p != '\0') && isspace(*p)) {
Expand All @@ -44,6 +48,106 @@ static char *skipNonWhitespace(char *p) {
return p;
}

#if defined(_AIX)
/* AIX does not understand '/proc/self' - it requires the real process ID */
#define FD_DIR aix_fd_dir
#define DIR DIR64
#define dirent dirent64
#define opendir opendir64
#define readdir readdir64
#define closedir closedir64
#elif defined(_ALLBSD_SOURCE)
#define FD_DIR "/dev/fd"
#else
#define FD_DIR "/proc/self/fd"
#endif

// Closes every file descriptor that is listed as a directory
// entry in "/proc/self/fd" (or its equivalent). Standard
// input/output/error file descriptors will not be closed
// by this function. This function returns 0 on failure
// and 1 on success.
int
closeDescriptors(void)
{
DIR *dp;
struct dirent *dirp;
/* leave out standard input/output/error descriptors */
int from_fd = STDERR_FILENO + 1;

/* We're trying to close all file descriptors, but opendir() might
* itself be implemented using a file descriptor, and we certainly
* don't want to close that while it's in use. We assume that if
* opendir() is implemented using a file descriptor, then it uses
* the lowest numbered file descriptor, just like open(). So
* before calling opendir(), we close a couple explicitly, so that
* opendir() can then use these lowest numbered closed file
* descriptors afresh. */

close(from_fd); /* for possible use by opendir() */
close(from_fd + 1); /* another one for good luck */
from_fd += 2; /* leave out the 2 we just closed, which the opendir() may use */

#if defined(_AIX)
/* set FD_DIR for AIX which does not understand '/proc/self' - it
* requires the real process ID */
char aix_fd_dir[32]; /* the pid has at most 19 digits */
snprintf(aix_fd_dir, 32, "/proc/%d/fd", getpid());
#endif

if ((dp = opendir(FD_DIR)) == NULL) {
ERROR_MESSAGE(("failed to open dir %s while determining"
" file descriptors to close for process %d",
FD_DIR, getpid()));
return 0; // failure
}

while ((dirp = readdir(dp)) != NULL) {
if (!isdigit(dirp->d_name[0])) {
continue;
}
const long fd = strtol(dirp->d_name, NULL, 10);
if (fd <= INT_MAX && fd >= from_fd) {
(void)close((int)fd);
}
}

(void)closedir(dp);

return 1; // success
}

// Does necessary housekeeping of a forked child process
// (like closing copied file descriptors) before
// execing the child process. This function never returns.
void
forkedChildProcess(const char *file, char *const argv[])
{
/* Close all file descriptors that have been copied over
* from the parent process due to fork(). */
if (closeDescriptors() == 0) { /* failed, close the old way */
/* Find max allowed file descriptors for a process
* and assume all were opened for the parent process and
* copied over to this child process. We close them all. */
const rlim_t max_fd = sysconf(_SC_OPEN_MAX);
JDI_ASSERT(max_fd != (rlim_t)-1); // -1 represents error
/* close(), that we subsequently call, takes only int values */
JDI_ASSERT(max_fd <= INT_MAX);
/* leave out standard input/output/error file descriptors */
rlim_t i = STDERR_FILENO + 1;
ERROR_MESSAGE(("failed to close file descriptors of"
" child process optimally, falling back to closing"
" %d file descriptors sequentially", (max_fd - i + 1)));
for (; i < max_fd; i++) {
(void)close(i);
}
}

(void)execvp(file, argv); /* not expected to return */

exit(errno); /* errno will have been set by the failed execvp */
}

int
dbgsysExec(char *cmdLine)
{
Expand Down Expand Up @@ -93,21 +197,11 @@ dbgsysExec(char *cmdLine)
argv[i] = NULL; /* NULL terminate */

if ((pid = fork()) == 0) {
/* Child process */
int i;
long max_fd;

/* close everything */
max_fd = sysconf(_SC_OPEN_MAX);
/*LINTED*/
for (i = 3; i < (int)max_fd; i++) {
(void)close(i);
}

(void)execvp(argv[0], argv);

exit(-1);
// manage the child process
forkedChildProcess(argv[0], argv);
}
// call to forkedChildProcess(...) will never return for a forked process
JDI_ASSERT(pid != 0);
jvmtiDeallocate(args);
jvmtiDeallocate(argv);
if (pid == pid_err) {
Expand Down

3 comments on commit a663248

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheRealMDoerr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on a663248 May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheRealMDoerr the backport was successfully created on the branch backport-TheRealMDoerr-a6632487 in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a6632487 from the openjdk/jdk repository.

The commit being backported was authored by Jaikiran Pai on 1 Feb 2024 and was reviewed by Gerard Ziemski, David Holmes and Chris Plummer.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-TheRealMDoerr-a6632487:backport-TheRealMDoerr-a6632487
$ git checkout backport-TheRealMDoerr-a6632487
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-TheRealMDoerr-a6632487

Please sign in to comment.