Build and deploy HelloFXMobile app to TestFlight#74
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds JavaFX support to the iOS mobile build infrastructure, enabling the creation of a HelloFXMobileApp that can be deployed to TestFlight. The PR extends the existing HelloWorld mobile app infrastructure by:
Changes:
- Added JavaFX native symbol declarations to symbol_keeper.cpp for iOS static linking
- Created a comprehensive local build script (script_sdk.sh) for iterative development without CI/CD
- Added HelloFXMobileApp with JavaFX UI sources, project configuration, and build scripts
- Updated GitHub workflows to build OpenJDK with JavaFX modules and bundle JavaFX libraries in xcframework
- Enhanced documentation with local build instructions and HelloFX app details
Reviewed changes
Copilot reviewed 38 out of 42 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| openjdk-ext/src/hotspot/symbol_keeper.cpp | Adds JavaFX JNI symbol declarations and removes verbose debug logging |
| local/script_sdk.sh | New comprehensive build script for local SDK and framework creation with JavaFX |
| docs/patches/symbol-keeper.md | Fixed spelling and improved formatting |
| docs/local.md | New documentation for local development workflow |
| docs/helloworld.md | Updated with clarifications and reference to automated scripts |
| docs/ga/*.md | Updated documentation for GitHub Actions workflows |
| appFX/* | New HelloFXMobileApp sources, configuration, and build scripts |
| app/script.sh | Enhanced with local mode support and extracted HelloWorld.java |
| app/sample/HelloWorld.java | Extracted HelloWorld source to separate file |
| .github/workflows/*.yml | Updated to build JavaFX modules and add Slack notifications |
| .github/scripts/build-xcframework.sh | Added JavaFX libraries to xcframework build |
| .github/patches/debug-ios-patch.diff | New patch for enabling iOS debug flags in JavaFX |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| loadfunctions(); | ||
| fprintf(stderr, "Create JavaVM\n"); | ||
| jint res = JNI_CreateJavaVM(&jvm, (void **)&env, &vm_args); | ||
| free(options[0].optionString); |
There was a problem hiding this comment.
strdup allocates memory for each string (1 string here, and 5 strings in appFX/main), passed down to the JVM creation, via vm_args, but then it is never freed, causing a small memory leak upon every launch of the application. Once the JVM is created it is safe to free them.
| -(void)startJava:(NSDictionary *)launchOptions { | ||
| pthread_t thread; | ||
| pthread_create(&thread, NULL, launchJava, NULL); | ||
| pthread_detach(thread); |
There was a problem hiding this comment.
why do you detach this here?
There was a problem hiding this comment.
It causes a memory leak otherwise: when the thread finishes execution its resources are not freed.
Not detaching it will be required if the thread runs during the whole app lifetime or if later on we do pthread_join(thread), but in this case it is only for launch time.
It has a bigger impact than the free(string) issue in main.
Calling detach allows reclaiming its resources when it exits.
But maybe, a better option is to use the attribute:
pthread_t thread;
pthread_attr_t attr;
pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
pthread_create(&thread, &attr, launchJava, NULL);
pthread_attr_destroy(&attr);
What do you think?
Fixes #72