Conversation
bretambrose approved these changes May 28, 2022
| // for old instances of the .dll and try to delete them. If another | ||
| // process is still using the .dll, the delete will fail, which is fine. | ||
| String os = getOSIdentifier(); | ||
| if (os == "windows") { |
Contributor
There was a problem hiding this comment.
Feel like platform id shouldn't be a string, but that's a larger change.
Contributor
There was a problem hiding this comment.
Another great reason to make it not-a-string!
Contributor Author
There was a problem hiding this comment.
I started making it not-a-string but it was turning into a big refactor so going with .equals() as suggested by Dombo
sbSteveK approved these changes May 31, 2022
graebm added a commit to aws/aws-iot-device-sdk-java-v2 that referenced this pull request May 31, 2022
Update to latest aws-crt. Contains [fix](awslabs/aws-crt-java#493) that, on startup, deletes any .dll files it finds from previous runs of the library.
graebm added a commit to aws/aws-iot-device-sdk-java-v2 that referenced this pull request May 31, 2022
Update to latest aws-crt. Contains [fix](awslabs/aws-crt-java#493) that, on startup, deletes any .dll files it finds from previous runs of the library.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ISSUE: On Windows, the .dll file extracted to the temp directory was never being deleted. And repeated runs of the program were filling the disk with old .dll files.
DIAGNOSIS: We were using File.deleteOnExit() to clean up the shared lib, which is working OK on other platforms, but not on Windows. On Windows, files cannot be deleted while they're in use, and it appears that Java doesn't try to "unload" the .dll, so it's apparently impossible for a Java process to delete a .dll that it's using.
I did an experiment with a dead-simple JNI library. I simply loaded it, and called
File.deleteOnExit(). That .dll didn't get deleted either. So it's not just a problem with our .dll being very complex.DESCRIPTION OF CHANGES: On Windows only, during startup, scan the temp dir for old instances of
AWSCRT_*aws-crt-jni.dlland try to delete them.SIDE-NOTE:
I realized that on non-Windows platforms, the shared-lib won't be deleted on exit if Java dies suddenly (crash in C, OS terminated process, etc). But on non-Windows platforms you can delete the shared-lib from disk immediately after loading it, you don't need to wait until the process exits. I did experiments on Mac and this works. But I left it out of this PR because this is an emergency fix for windows and I don't want to introduce non-necessary changes.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.