{"action":"closed","issue":{"url":"https://api.github.com/repos/takari/aether-connector-okhttp/issues/15","repository_url":"https://api.github.com/repos/takari/aether-connector-okhttp","labels_url":"https://api.github.com/repos/takari/aether-connector-okhttp/issues/15/labels{/name}","comments_url":"https://api.github.com/repos/takari/aether-connector-okhttp/issues/15/comments","events_url":"https://api.github.com/repos/takari/aether-connector-okhttp/issues/15/events","html_url":"https://github.com/takari/aether-connector-okhttp/issues/15","id":339970431,"node_id":"MDU6SXNzdWUzMzk5NzA0MzE=","number":15,"title":"Race condition in AetherRepositoryConnector","user":{"login":"helospark","id":14263031,"node_id":"MDQ6VXNlcjE0MjYzMDMx","avatar_url":"https://avatars3.githubusercontent.com/u/14263031?v=4","gravatar_id":"","url":"https://api.github.com/users/helospark","html_url":"https://github.com/helospark","followers_url":"https://api.github.com/users/helospark/followers","following_url":"https://api.github.com/users/helospark/following{/other_user}","gists_url":"https://api.github.com/users/helospark/gists{/gist_id}","starred_url":"https://api.github.com/users/helospark/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/helospark/subscriptions","organizations_url":"https://api.github.com/users/helospark/orgs","repos_url":"https://api.github.com/users/helospark/repos","events_url":"https://api.github.com/users/helospark/events{/privacy}","received_events_url":"https://api.github.com/users/helospark/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-07-10T19:07:31Z","updated_at":"2018-07-25T19:00:49Z","closed_at":"2018-07-25T19:00:49Z","author_association":"CONTRIBUTOR","body":"## Background\r\nI'm investigating some jar file corruption occurring in my .m2 when using Eclipse's m2e plugin to import Maven projects.\r\n\r\nUsually a jar file, that is larger than the original jar is saved in the .m2 folder. But sometimes the jar file is 0 bytes, other times, it smaller than the actual jar. Besides jars, pom.xmls are often getting corrupted too.\r\nChecking the sha1 shows that the file is indeed corrupt.\r\n\r\nThis causing both Eclipse build, and later actual Maven build to fail for variety of reasons, like:\r\n - Invalid ZIP file, when opening the jar\r\n - Unparseable pom.xml, if the corruption happened in an XML\r\n - NoSuchMethod, NoSuchClass exception, if there is no direct compile time reliance of these jars\r\n - Not starting Spring Boot autoconfigurations\r\n - and many others...\r\n\r\nThese are especially hard bugs to find.\r\nMore details in the Eclipse bugs:\r\nhttps://bugs.eclipse.org/bugs/show_bug.cgi?id=534228\r\nhttps://bugs.eclipse.org/bugs/show_bug.cgi?id=514691\r\n\r\n## Repro\r\nI have made an integration test that shows the the corruption reliably, you can find it here:\r\nhttps://github.com/takari/aether-connector-okhttp/compare/master...helospark:concurrency-test\r\n\r\nThis test simulates an Artifactory interaction with aether-connection, without actual remote calls.\r\nStarts a few threads that are trying to download the same file.\r\nIt retries the test 10 times, to make sure the concurrency issues are shown.\r\n\r\n## Cause\r\nThe cause is the \"resume download\" feature built into the connector. Based on some debugging the following happens:\r\n - Two threads enter, trying to download the same file\r\n - First thread creates a tmp file (aether-...-in-progress) , and starts to write into the file\r\n - Second thread find the tmp file, and \"thinks\" the previous download get interrupted, so it tries to download bytes after the file's current content (resuming the download)\r\n - Now 2 threads are writing to the same file, usually result in a file larger than the original\r\n - After the file is downloaded, checksum may fail (but it can happen, that the first threads's checksum check happens before the second thread flushes to the file).\r\n - If the checksum behavior set to \"warn\", then the corrupt jar file just gets renamed to the final name\r\n - If the checksum behavior set to \"fail\", then the corrupt jar does not get renamed, however during the next download, the corrupt tmp \"in-progress\" file is found, and the download will fail again, because this tmp file is never removed. Only manually clearing this from .m2 makes this download work.\r\n\r\n## Possible fixes\r\n  - Since most artifacts are relatively small, I don't really see the benefit of continuing a download. I think the simplest option would be to remove this feature. Or make it switchable.\r\n - Another possibility is to not ignore checksum problems, instead redownload without the corrupt tmp file. Though this would create the problem, that multiple threads are retrying the same download. \r\n - Tmp files should always be deleted at the end of the download or ignored during the next, whether successful or not, otherwise retrying the download will also fail every time.\r\n - In-JVM solution to prevent the same file to download by multiple threads (eg. some locking mechanism that keeps tracks of which files are currently downloading, and prevents another thread entering with the same file). \r\n\r\n## Further considerations\r\n - While removing the resume download feature locally, I have not been able to see any more jar corruption, there is still a small chance, that the FileProcessor.move method is not actually atomic. It uses the \r\n   - File.renameTo to move the file, which this may not be atomic depending on the OS\r\n   - If the renameTo fails, than it copies the content using FileOutputStream, which is by default not atomic. Though it always clears the destination file. I have not been able to get this to corrupt the file. If we want absolute security, we could use FileLocks here.\r\n\r\nAs long as the tmp files are always deleted (or not using during init), I don't think, we have to worry about these so much, because the likelihood of this causing an issue seems small, and a retry solves it."}}
{"actor":{"display_login":"fbricon"}}