On Fri, 1 Dec 2023 11:21:59 GMT, Maxim Kartashev <mkartas...@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 46: > >> 44: } >> 45: >> 46: static volatile gboolean hasPipewireFailed = FALSE; > > I wonder why does `hasPipewireFailed` have to be `volatile`? @mkartashev hey, a fellow Sun comrade! (i still have my Sun badge too) :) This is for similar reasons to the existing 'sessionClosed' being volatile ie bc multiple threads can read/write it and in this particular case a limited scope optimizatiion compiler might decide to otherwise optimize away the 'if' block in this loop https://github.com/openjdk/jdk/pull/16794/commits/6805f72d4b7f33d5bfa0ae5742122d377345c6e2#diff-3ba5df79cdd3da36b21bf29b4e8de462dd61e6a21dfe0816e4d84c18bbfb76b2R927 In practice tho I dont think modern compilers would do that but since there is no real cost of using volatile in this case as there is nothing practical to gain by optimizing around it, its best to explicitly tell the compiler to not mess with it at all than to rely on any assumption that it just would not as those optimizations are implementation dependent. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16794#discussion_r1412085551
