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

[Anton Bobrov]

On Wed, 6 Dec 2023 19:20:22 GMT, Phil Race <prr at openjdk.org> wrote:

>> Anton Bobrov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8321176: remove RuntimeException and address review comments
>
> src/java.desktop/unix/native/libawt_xawt/awt/screencast_pipewire.c line 102:
> 
>> 100:             if (screenProps->data->stream) {
>> 101:                 fp_pw_thread_loop_lock(pw.loop);
>> 102:                 fp_pw_stream_disconnect(screenProps->data->stream);
> 
> I had this disconnect inside the lock in an earlier PR 
> https://github.com/openjdk/jdk/pull/14428/files?diff=unified&w=0#diff-3ba5df79cdd3da36b21bf29b4e8de462dd61e6a21dfe0816e4d84c18bbfb76b2R92
> but then it was moved out of the lock in another PR to fix a bug https://bugs.openjdk.org/browse/JDK-8310334
> https://github.com/openjdk/jdk/pull/15250/files#diff-3ba5df79cdd3da36b21bf29b4e8de462dd61e6a21dfe0816e4d84c18bbfb76b2
> 
> 
> So how can it be right to put it back inside the lock ?

@prrace the related previous fixes didnt really fix anything and just re-shuffled things around to dodge specific problems encountered. the libpipewire API locking and signaling has been fundamentally broken before my patch for JDK-8320655. this is understandable given libpipewire lack of proper documentation in that area and no clear specs on when and how locking and signaling should be done.

this particular case you refer to you need to lock around disconnect bc if you don't there is a chance the disconnect API can fail here

https://github.com/PipeWire/pipewire/blob/master/src/pipewire/thread-loop.c#L100

since this is recursive lock, taking it explicitly before calling disconnect ensures this does not happen. if you test the old implementation with libpipewire debug enabled you would see it constantly failing in related API and complaining functions being called in the "wrong context", including this code in question.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16978#discussion_r1417918800