Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIO-related tests fail on Windows #12

Closed
eric239 opened this issue Nov 29, 2014 · 5 comments
Closed

NIO-related tests fail on Windows #12

eric239 opened this issue Nov 29, 2014 · 5 comments

Comments

@eric239
Copy link

eric239 commented Nov 29, 2014

There is a known issue concerning jvm's inability to support deletion of memory mapped files that is described here: http://stackoverflow.com/questions/4179145/release-java-file-lock-in-windows

This results in NIO-related tests failing on windows only

There are hacks like executing

System.gc();
Thread.sleep(1000);

prior to the deletion attempt.

The original thread started in issue #7.

I think it won't be right to change the main code (i.e not to use NIO) due to this problem. Perhaps all the tests that try to delete from the FS can call a common wrapper method which can check whether OS is a Windows flavor, and if so, implement the above hack.

@FelixGV
Copy link
Contributor

FelixGV commented Nov 29, 2014

Thanks Eric!

I agree with you: we should implement a workaround that only affects the tests when run on a problematic platform. Instead of explicitly checking for which OS we're running on, we may want to just catch the exception and then retry after invoking the gc and sleep.

-F

@eric239
Copy link
Author

eric239 commented Nov 29, 2014

Well, as it happens

Instead of explicitly checking for which OS we're running on, we may want to just catch the exception and then retry after invoking the gc and sleep.

may be somewhat problematic b/c the source of the issue lies with the closing (and no exception is produced there) whereas it manifests in either attempting to delete or attempting to reopen the same file/lock. Any concerns with placing "if this os is windows then GC and sleep" block in LocalFileSystemRepository#close() other than aesthetic ones? That way it's in one place, right after the [broken on windows] code. I think the intent will be clear enough.

@FelixGV
Copy link
Contributor

FelixGV commented Nov 29, 2014

I haven't tested this locally so I may not properly grasp where and how the problem manifests.

I was thinking we could put the Windows work around in TestLocalFileSystemRepository#rmDir(File dir).

Would that be sufficient?

-F

@eric239
Copy link
Author

eric239 commented Nov 30, 2014

Yep, that'll do too, though there will remain a possibility of test-like flow (close, attempt to reopen/delete) happening in main (non-test) code. I'll put in the spot you mentioned for the time being.

eric239 pushed a commit to eric239/schema-repo that referenced this issue Nov 30, 2014
FelixGV pushed a commit to FelixGV/schema-repo that referenced this issue Dec 2, 2014
@FelixGV
Copy link
Contributor

FelixGV commented Dec 2, 2014

The fix for this has been merged.

Thanks a lot!

@FelixGV FelixGV closed this as completed Dec 2, 2014
FelixGV pushed a commit to FelixGV/schema-repo that referenced this issue Dec 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants