[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Integrated: 8321176: [Screencast] make a second attempt on screencast failure
From:       Anton Bobrov <duke () openjdk ! org>
Date:       2023-12-11 8:32:27
Message-ID: K7aAn4XBvhQlYKf2dYx74cFb0bR4RleasID-uLODzMI=.5c00679c-59b6-49b1-aac8-c0db5231a347 () github ! com
[Download RAW message or body]

On Tue, 5 Dec 2023 16:48:55 GMT, Anton Bobrov <duke@openjdk.org> wrote:

> 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.

This pull request has now been integrated.

Changeset: 92fd490f
Author:    Anton Bobrov <antbob@users.noreply.github.com>
Committer: Alexander Zvegintsev <azvegint@openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/92fd490f22f690ff7698182658363b7035bcc3bf
                
Stats:     76 lines in 2 files changed: 52 ins; 16 del; 8 mod

8321176: [Screencast] make a second attempt on screencast failure

Reviewed-by: azvegint, prr

-------------

PR: https://git.openjdk.org/jdk/pull/16978


[prev in list] [next in list] [prev in thread] [next in thread]