Skip to content

Commit

Permalink
8274856: Failing jpackage tests with fastdebug/release build
Browse files Browse the repository at this point in the history
Reviewed-by: almatvee, herrick
  • Loading branch information
Alexey Semenyuk committed Nov 15, 2021
1 parent 9046077 commit fe45835
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 46 deletions.
125 changes: 119 additions & 6 deletions src/jdk.jpackage/linux/native/applauncher/LinuxLauncher.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
#include <dlfcn.h>
#include <errno.h>
#include <linux/limits.h>
#include <sys/wait.h>
#include <unistd.h>
#include <stddef.h>
#include <libgen.h>
#include "JvmLauncher.h"
#include "LinuxPackage.h"
Expand All @@ -43,7 +45,7 @@ static int appArgc;
static char **appArgv;


static JvmlLauncherData* initJvmlLauncherData(void) {
static JvmlLauncherData* initJvmlLauncherData(int* size) {
char* launcherLibPath = 0;
void* jvmLauncherLibHandle = 0;
JvmlLauncherAPI_GetAPIFunc getApi = 0;
Expand Down Expand Up @@ -86,7 +88,7 @@ static JvmlLauncherData* initJvmlLauncherData(void) {
goto cleanup;
}

result = jvmLauncherCreateJvmlLauncherData(api, jvmLauncherHandle);
result = jvmLauncherCreateJvmlLauncherData(api, jvmLauncherHandle, size);
/* Handle released in jvmLauncherCreateJvmlLauncherData() */
jvmLauncherHandle = 0;

Expand Down Expand Up @@ -131,18 +133,129 @@ static int launchJvm(JvmlLauncherData* cfg) {
}


static void closePipeEnd(int* pipefd, int idx) {
if (pipefd[idx] >= 0) {
close(pipefd[idx]);
pipefd[idx] = -1;
}
}


static void initJvmlLauncherDataPointers(void* baseAddress,
JvmlLauncherData* jvmLauncherData) {
ptrdiff_t offset = (char*)jvmLauncherData - (char*)baseAddress;
int i;

jvmLauncherData->jliLibPath += offset;
jvmLauncherData->jliLaunchArgv =
(char**)((char*)jvmLauncherData->jliLaunchArgv + offset);
jvmLauncherData->envVarNames =
(char**)((char*)jvmLauncherData->envVarNames + offset);
jvmLauncherData->envVarValues =
(char**)((char*)jvmLauncherData->envVarValues + offset);

for (i = 0; i != jvmLauncherData->jliLaunchArgc; i++) {
jvmLauncherData->jliLaunchArgv[i] += offset;
}

for (i = 0; i != jvmLauncherData->envVarCount; i++) {
jvmLauncherData->envVarNames[i] += offset;
jvmLauncherData->envVarValues[i] += offset;
}
}


int main(int argc, char *argv[]) {
int pipefd[2];
pid_t cpid;
int exitCode = STATUS_FAILURE;
JvmlLauncherData* jvmLauncherData;
JvmlLauncherData* jvmLauncherData = 0;
int jvmLauncherDataBufferSize = 0;

appArgc = argc;
appArgv = argv;

jvmLauncherData = initJvmlLauncherData();
if (jvmLauncherData) {
if (pipe(pipefd) == -1) {
JP_LOG_ERRNO;
return exitCode;
}

cpid = fork();
if (cpid == -1) {
JP_LOG_ERRNO;
} else if (cpid == 0) /* Child process */ {
/* Close unused read end */
closePipeEnd(pipefd, 0);

jvmLauncherData = initJvmlLauncherData(&jvmLauncherDataBufferSize);
if (jvmLauncherData) {
/* Buffer size */
if (write(pipefd[1], &jvmLauncherDataBufferSize,
sizeof(jvmLauncherDataBufferSize)) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}
if (jvmLauncherDataBufferSize) {
/* Buffer address */
if (write(pipefd[1], &jvmLauncherData,
sizeof(jvmLauncherData)) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}
/* Buffer data */
if (write(pipefd[1], jvmLauncherData,
jvmLauncherDataBufferSize) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}
}
}

exitCode = 0;
} else if (cpid > 0) {
void* baseAddress = 0;

/* Close unused write end */
closePipeEnd(pipefd, 1);

if (read(pipefd[0], &jvmLauncherDataBufferSize,
sizeof(jvmLauncherDataBufferSize)) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}

if (jvmLauncherDataBufferSize == 0) {
JP_LOG_ERRNO;
goto cleanup;
}

if (read(pipefd[0], &baseAddress, sizeof(baseAddress)) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}

jvmLauncherData = malloc(jvmLauncherDataBufferSize);
if (!jvmLauncherData) {
JP_LOG_ERRNO;
goto cleanup;
}

if (read(pipefd[0], jvmLauncherData,
jvmLauncherDataBufferSize) == -1) {
JP_LOG_ERRNO;
goto cleanup;
}

closePipeEnd(pipefd, 0);
wait(NULL); /* Wait child process to terminate */

initJvmlLauncherDataPointers(baseAddress, jvmLauncherData);
exitCode = launchJvm(jvmLauncherData);
free(jvmLauncherData);
}

cleanup:
closePipeEnd(pipefd, 0);
closePipeEnd(pipefd, 1);
free(jvmLauncherData);
return exitCode;
}
12 changes: 2 additions & 10 deletions src/jdk.jpackage/linux/native/libapplauncher/LinuxLauncherLib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,9 @@ void launchApp() {
launchInfo = (tstrings::any() << thisHash).str();
}

JP_TRY;
if (0 != setenv(_JPACKAGE_LAUNCHER.c_str(), launchInfo.c_str(), 1)) {
JP_THROW(tstrings::any() << "setenv(" << _JPACKAGE_LAUNCHER
<< ", " << launchInfo << ") failed. Error: " << lastCRTError());
} else {
LOG_TRACE(tstrings::any() << "Set "
<< _JPACKAGE_LAUNCHER << "=[" << launchInfo << "]");
}
JP_CATCH_ALL;

jvmLauncher = appLauncher.createJvmLauncher();

jvmLauncher->addEnvVariable(_JPACKAGE_LAUNCHER, launchInfo);
}

} // namespace
Expand Down
6 changes: 3 additions & 3 deletions src/jdk.jpackage/share/native/applauncher/AppLauncher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@ Jvm* AppLauncher::createJvmLauncher() const {
PropertyName::arguments, args);
}

std::unique_ptr<Jvm> jvm(new Jvm());

if (!libEnvVariableContainsAppDir()) {
SysInfo::setEnvVariable(libEnvVarName, SysInfo::getEnvVariable(
(*jvm).addEnvVariable(libEnvVarName, SysInfo::getEnvVariable(
std::nothrow, libEnvVarName)
+ FileUtils::pathSeparator
+ appDirPath);
}

std::unique_ptr<Jvm> jvm(new Jvm());

(*jvm)
.setPath(findJvmLib(cfgFile, defaultRuntimePath, jvmLibNames))
.addArgument(launcherPath);
Expand Down
99 changes: 75 additions & 24 deletions src/jdk.jpackage/share/native/applauncher/JvmLauncher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ void Jvm::launch() {
JvmlLauncherAPI* api = jvmLauncherGetAPI();

AutoJvmlLauncherData jld(jvmLauncherCreateJvmlLauncherData(api,
jlh.release()));
jlh.release(), 0));

LOG_TRACE(tstrings::any() << "JVM library: \"" << jvmPath << "\"");

Expand All @@ -215,11 +215,20 @@ void Jvm::launch() {
}


void Jvm::setEnvVariables() {
for (size_t i = 0; i != envVarNames.size(); i++) {
SysInfo::setEnvVariable(envVarNames.at(i), envVarValues.at(i));
}
}


namespace {

struct JliLaunchData {
std::string jliLibPath;
std::vector<std::string> args;
tstring_array envVarNames;
tstring_array envVarValues;

int initJvmlLauncherData(JvmlLauncherData* ptr, int bufferSize) const {
int minimalBufferSize = initJvmlLauncherData(0);
Expand All @@ -230,6 +239,29 @@ struct JliLaunchData {
}

private:
template <class T>
static char* copyStrings(const std::vector<T>& src,
JvmlLauncherData* ptr, const size_t offset, char* curPtr) {
char** strArray = 0;
if (ptr) {
strArray = *reinterpret_cast<char***>(
reinterpret_cast<char*>(ptr) + offset);
}

for (size_t i = 0; i != src.size(); i++) {
const size_t count = (src[i].size() + 1 /* trailing zero */)
* sizeof(typename T::value_type);
if (ptr) {
std::memcpy(curPtr, src[i].c_str(), count);
strArray[i] = curPtr;
}

curPtr += count;
};

return curPtr;
}

int initJvmlLauncherData(JvmlLauncherData* ptr) const {
// Store path to JLI library just behind JvmlLauncherData header.
char* curPtr = reinterpret_cast<char*>(ptr + 1);
Expand All @@ -243,26 +275,39 @@ struct JliLaunchData {
curPtr += count;
}

// Next write array of char* pointing to JLI lib arg strings.
// Write array of char* pointing to JLI lib arg strings.
if (ptr) {
ptr->jliLaunchArgv = reinterpret_cast<char**>(curPtr);
ptr->jliLaunchArgc = (int)args.size();
// Add terminal '0' arg.
ptr->jliLaunchArgv[ptr->jliLaunchArgc] = 0;
}

// Skip memory occupied by char* array.
// Skip memory occupied by JvmlLauncherData::jliLaunchArgv array.
curPtr += sizeof(char*) * (args.size() + 1 /* terminal '0' arg */);
// Store JLI lib arg strings.
curPtr = copyStrings(args, ptr,
offsetof(JvmlLauncherData, jliLaunchArgv), curPtr);

// Store array of strings.
for (size_t i = 0; i != args.size(); i++) {
const size_t count = (args[i].size() + 1 /* trailing zero */);
if (ptr) {
std::memcpy(curPtr, args[i].c_str(), count);
ptr->jliLaunchArgv[i] = curPtr;
}
curPtr += count;
};
// Write array of char* pointing to env variable name strings.
if (ptr) {
ptr->envVarNames = reinterpret_cast<TCHAR**>(curPtr);
ptr->envVarCount = (int)envVarNames.size();
}
// Skip memory occupied by JvmlLauncherData::envVarNames array.
curPtr += sizeof(TCHAR*) * envVarNames.size();
// Store env variable names.
curPtr = copyStrings(envVarNames, ptr,
offsetof(JvmlLauncherData, envVarNames), curPtr);

// Write array of char* pointing to env variable value strings.
if (ptr) {
ptr->envVarValues = reinterpret_cast<TCHAR**>(curPtr);
}
// Skip memory occupied by JvmlLauncherData::envVarValues array.
curPtr += sizeof(TCHAR*) * envVarValues.size();
// Store env variable values.
curPtr = copyStrings(envVarValues, ptr,
offsetof(JvmlLauncherData, envVarValues), curPtr);

const size_t bufferSize = curPtr - reinterpret_cast<char*>(ptr);
if (ptr) {
Expand All @@ -276,24 +321,30 @@ struct JliLaunchData {
}
};

} // namespace

JvmlLauncherHandle Jvm::exportLauncher() const {
std::unique_ptr<JliLaunchData> result(new JliLaunchData());

result->jliLibPath = tstrings::toUtf8(jvmPath);

void copyStringArray(const tstring_array& src, std::vector<std::string>& dst) {
#ifdef TSTRINGS_WITH_WCHAR
{
tstring_array::const_iterator it = args.begin();
const tstring_array::const_iterator end = args.end();
tstring_array::const_iterator it = src.begin();
const tstring_array::const_iterator end = src.end();
for (; it != end; ++it) {
result->args.push_back(tstrings::toACP(*it));
dst.push_back(tstrings::toACP(*it));
}
}
#else
result->args = args;
dst = src;
#endif
}

} // namespace

JvmlLauncherHandle Jvm::exportLauncher() const {
std::unique_ptr<JliLaunchData> result(new JliLaunchData());

result->jliLibPath = tstrings::toUtf8(jvmPath);

copyStringArray(args, result->args);
result->envVarNames = envVarNames;
result->envVarValues = envVarValues;

return result.release();
}
Expand Down
Loading

3 comments on commit fe45835

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on fe45835 Jul 18, 2022

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on fe45835 Jul 18, 2022

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-fe45835f in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-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 fe45835f from the openjdk/jdk repository.

The commit being backported was authored by Alexey Semenyuk on 15 Nov 2021 and was reviewed by Alexander Matveev and Andy Herrick.

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/jdk17u-dev:

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

Please sign in to comment.