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

New NEXRAD related failure #42

Closed
lesserwhirls opened this issue Jul 31, 2019 · 5 comments · Fixed by #44
Closed

New NEXRAD related failure #42

lesserwhirls opened this issue Jul 31, 2019 · 5 comments · Fixed by #44

Comments

@lesserwhirls
Copy link
Collaborator

New failure in ucar.nc2.iosp.nexrad2.TestNexrad2.testRead on Jenkins (possibly related to #37).

java.io.IOException: java.nio.channels.ClosedChannelException

	at ucar.nc2.NetcdfFile.open(NetcdfFile.java:500)
	at ucar.nc2.dataset.NetcdfDataset.openOrAcquireFile(NetcdfDataset.java:713)
	at ucar.nc2.dataset.NetcdfDataset.openFile(NetcdfDataset.java:580)
	at ucar.nc2.iosp.nexrad2.TestNexrad2$MyAct.doAct(TestNexrad2.java:56)
	at ucar.unidata.util.test.TestDir.actOnAll(TestDir.java:263)
	at ucar.unidata.util.test.TestDir.actOnAll(TestDir.java:213)
	at ucar.nc2.iosp.nexrad2.TestNexrad2.testRead(TestNexrad2.java:34)

Does not consistently bomb out on a particular file or after processing particular number of files.

@JohnLCaron
Copy link
Collaborator

My apologies, I obviously didnt get that refactor right. Its intermittent because it deals with the case of producing an uncompressed file, which it then leaves in place.

The logic is rather fiendish, probably we should roll the PR back until I can fix. Umm, how do you do that?

@lesserwhirls
Copy link
Collaborator Author

No need to apologize! That's why we have tests :-) I can roll back just the nexrad changes, but I think I found the underlying problem. As part of your PR, you moved to using try-with-resource here:

try (RandomAccessFile outputRaf = new RandomAccessFile(ufilename, "rw")) {

Good stuff. Within that try-with-resource block, we acquire a lock. Cool. The whole try-with-resource block concludes with a finally:

} finally {
if (lock != null) {
lock.release();
}
} // try-with-resource
}

but by that point, we're out of the try block and the source has been closed, so the channel is closed and sad times ensue. The lock isn't valid anymore (lock.isValid() = false) by the time we do the null check, as it appears to get released when we move out of the try block, so maybe we don't need a finally block anymore? Or maybe add a log message in case we get there and the lock is still valid somehow?

} finally { 
  if (lock != null && lock.isValid()) { 
      log.error("Uncompresed file is closed but the lock is still valid. Why is you the way you is, file lock?");
  } 
} // try-with-resource 

@JohnLCaron
Copy link
Collaborator

I dont understand then why it wouldnt fail every time?

@lesserwhirls
Copy link
Collaborator Author

I think it's because the lock is only obtained when decompressing, otherwise lock=null. The test bombs out on the first file it encounters that needs to be decompressed.

@JohnLCaron
Copy link
Collaborator

Ok, I have it in the debugger and I agree with your analysis. I think we could just ditch the finally block all together.

lesserwhirls added a commit to lesserwhirls/netcdf-java that referenced this issue Aug 2, 2019
lesserwhirls added a commit to lesserwhirls/netcdf-java that referenced this issue Aug 2, 2019
lesserwhirls added a commit to lesserwhirls/netcdf-java that referenced this issue Aug 2, 2019
lesserwhirls added a commit that referenced this issue Aug 3, 2019
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

Successfully merging a pull request may close this issue.

2 participants