- Notifications
You must be signed in to change notification settings - Fork44
remove redundant File.length() check to fix race condition#60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
IMHO this patch does not FIX the race condition, rather just ignore it. I think the check provides a valuable fail-fast mechanism to detect possible problems (as you just did) but removing it won't solve the race condition. The concurrency issue is at a higher level and it is there where it should be fixed. |
the issue should be safe to ignore since multiple writers to the same file will each succeed in writing their own full copy of the file (on unix) whereas on windows they will either block or fail early. If you don't wish to merge please can you suggest a way forward?
|
I suggest to translate this discussion to the maven developer mailing list to get more answers. |
actually, here's another point on why I think the post-copy length check is flawed - it can never be thread safe / concurrency friendly, since there's no telling how much time may pass between completion of the file copy and the length check. Imagine a couple of not too fanciful scenarios:
On these grounds, I think the only sane strategy for checking in plexus-util has to be no stronger that a check on number of bytes written |
@slawekjaranowski wdyt? |
See#47