On Thu, 30 Nov 2023 21:41:53 GMT, Alexander Zvegintsev <azveg...@openjdk.org> wrote:
>> This patch addresses the issues described in the >> https://bugs.openjdk.org/browse/JDK-8320655 by fixing the proper locking and >> signalling around libpipewire thread loop condition variables and also >> fixing libpipewire error detection and signalling and propagation to the >> screencast API. This makes the screencast robot stable enough to >> consistently make it thru the entire javax/swing jtreg suite without hanging >> and also significantly reduces CPU consumption as there is no longer any >> burning spinners since they are now waiting on related conditions proper. > > src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 930: > >> 928: fp_pw_thread_loop_unlock(pw.loop); >> 929: releaseToken(env, jtoken, token); >> 930: return RESULT_ERROR; > > I think that the fix can be improved a bit. > Right now it just gives a black image on failure. We can try to add another > attempt to get the image we need. @azvegint Thanks for reviewing this Alexander! I dont think re-trying here is gonna work at all. The libpipewire docs leave a lot to be desired so I was using their code as reference for this patch and according to their current implementation if it gets to a state of core error or if it transitions to PW_STREAM_STATE_ERROR or PW_STREAM_STATE_UNCONNECTED from any other state that means a fatal (unrecoverable) error meaning you have to bail and start from scratch eg re-init, get new stream etc ie there is no transition from such state to a normal state where you could make a re-try. The problem with blank screen in this case stems from the API design here. Currently, when an error here the higher level API, as far as I can tell, only covers the permissions case by throwing security exception but completely ignores any other possible errors instead of propagating them to the caller, either as exception or return value, and so the caller gets an empty/black image on failure but is simply not aware of any encountered error/s and thus could incorrectly assume the data is legit. What should be done to address that is to propagate those errors all the way back to the API caller but doing so would require changing related API method signatures or introducing new throws that the callers would need to catch. I dunno about the stability levels and contracts on those API involved but if they are mendable I can look into changing them tho I think it is better to do it as separate patch. Tell me what you think. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16794#discussion_r1411840126
