> This patch adds re-try logic to libpipewire screencast error handling as > discussed in PR #16794 and also brings some additional error handling and > thread safety improvements. Specifically around cleanup order where incorrect > ordering lead to native memory corruption issues, and lock/unlock accounting > that while mostly harmless (with the current libpipewire implementation) did > pollute the stderr on jtreg tests, making some tests (expecting clean stderr) > to fail. > > The real major change here however is the throw of the RuntimeException which > can propagate to public > > java.awt.Robot. createMultiResolutionScreenCapture, createScreenCapture, > getPixelColor methods. I'm not sure the plain RuntimeException is the way to > go here so this is just a placeholder of sorts. A separate/specific runtime > exception can be created for this BUT *something* needs to be done here as > the current implementation fails to convey libpipewire failures to those > public API callers and since they have no way of detecting such errors > otherwise they have no way of knowing that the data returned by those API is > in fact invalid (eg black screens etc). The reason for using an unchecked > exception here is driven mainly by the following factors: > > 1) Since those are public API, their contracts can potentially make it > difficult to introduce specific additional checked exceptions or return > values (as appropriate) as those could potentially break existing API use. > > 2) The libpipewire errors of that kind are rare and usually indicate there is > something wrong with the desktop stack eg some fatal configuration or run > time error that is normally not supposed to happen and given this patch now > goes extra step re-trying on such failures it stands to reason runtime > unchecked exception makes sense when that fails as well. > > 3) Creating checked exceptions for such specific native implementation > dependent errors and propagating such exceptions thru the call tree does not > make much sense as most public API users won't even know how to handle them > without knowing native implementation specifics.
Anton Bobrov has updated the pull request incrementally with one additional commit since the last revision: 8321176: remove RuntimeException and address review comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16978/files - new: https://git.openjdk.org/jdk/pull/16978/files/8f79cac7..a473b9d2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16978&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16978&range=00-01 Stats: 11 lines in 2 files changed: 6 ins; 3 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16978.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16978/head:pull/16978 PR: https://git.openjdk.org/jdk/pull/16978