Throwing Away The Interrupt Flag by catching the InterruptedException

[JLLeitschuh]

There are at least 5 locations where this library simply catches an InterruptedException and does nothing with it.
https://github.com/bytedeco/javacv/search?utf8=%E2%9C%93&q=InterruptedException

http://www.ibm.com/developerworks/library/j-jtp05236/
How an InterruptedException should be handled in is detailed in the link above. In a nutshell, the InterruptedException should never be simply tossed away. You should always either propigate the exception or reset the threads interrupted flag.

[saudet]

Thanks, I'll look at that, eventually, but is there anything that isn't working because of that for you application?

[JLLeitschuh]

We are running the framegrabber in its own thread at present. When we want to stop the thread we expect that anything that may be blocking will respond correctly to an InterruptedException by either rethrowing it or by "preserving state" and allowing the interrupt flag to propigate.

This may have been causing our thread to deadlock in our application when trying to stop it because the interrupt flag was being eaten.

We have been a slew of various problems with cameras and I'm trying to figure out all of the potential deadlock locations.

Also, I would label this as a bug not an enhancement.

[saudet]

I see, so calling Thread.currentThread().interrupt() would be a satisfactory fix?

[JLLeitschuh]

Yes. I mean more ideal is passing the InterruptedException is better practice.

Well-behaved blocking library methods should be responsive to interruption and throw InterruptedException so they can be used within cancelable activities without compromising responsiveness.
Brian Goetz

But if you are trying to not break your API then yes. This is fine.

[saudet]

Sure, I rethrow the exception when it makes sense, but Thread.sleep() is mostly called to work around temporary failures that occur frequently, by retrying the operations. IMO, it's not something that should be exposed more than we have to. It's something the upstream libraries should fix, and then we wouldn't need to use Thread.sleep() in the first place.

[saudet]

Let me know if the commits above are alright! Thanks

[JLLeitschuh]

The only one that I'm worried about is this one: 6875b0e#diff-a47e0edeb3dfdcb531fe9fa18e016ef6R210

By calling start the frame grabber is fulfilling a promise that when the method returns without an exception then the grabber will be working.
Because by returning without an exception you are basically telling the thread and the user that the frame grabber has been started successfully when it really hasn't.

Thoughts?

[saudet]

Well, it probably doesn't work either if we exceed 100 counts either...

[JLLeitschuh]

So both cases should throw an exception probably. But just make sure you preserve the interrupt flag like you have.

[saudet]

Yeah, that sounds good. I've updated that in the latest commit above.

[JLLeitschuh]

That works. Cool!! Thanks for taking care of this. You guys are doing a great job with this library.