> 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

Reply via email to