From 9dfb90fb32fd9f30f067869338a9a290e95f2013 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 20:16:43 +0000 Subject: [PATCH 1/6] correctness: Add capacity and eviction bounds check to FrameCache * Move totalLoads++ into write lock to fix potential race condition * Add read locks to all metric getters in FrameCache Co-authored-by: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> --- .../cabrillo/tracker/video/FrameCache$1.class | Bin 0 -> 1030 bytes .../cabrillo/tracker/video/FrameCache.class | Bin 0 -> 5547 bytes .../cabrillo/tracker/video/FrameCache.java | 37 +++++++++++++----- 3 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class create mode 100644 src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class new file mode 100644 index 0000000000000000000000000000000000000000..9ea2b461de0756519bee9325d52f267df97e19f5 GIT binary patch literal 1030 zcmbVLT~8B16g^Y6U0Mn)1q2lpwP;(^)hA*~@Po8rV<9FrJn%N`4(-Txx0#t%@yQ=! z;)^dPsEH5$0DqM6?v|vG@YGFq?wK>0Gxwgk^Y`EHKLB3gaTX~|*|73RBg61KQXMDi zi%>@c<%xdxR7+1g9zRkt2qMQQ?wttb45cq3r>S^P>~OCu%Fh@wrYm*%DT%ZuRH^4M zjjWAa9y7=@EWhVN?hK3!oR$nvgulymcaQh!tSuQzvrVAlHV>-@IVxi=j{=GemX@Cc z!&If}W>La|jm12caE)Pe!n_#l9{<=*WSPtGqB3!XNw$>ZPUKUL1u9GI4|+#Jz2ip# zF~vP$x{?2u$JQF5G{d|Sy=W*JfiJXaghrh*tW;WKZ^)5y!q=;Z46|)RcSPK|Vzw>Tj;xwl&fUc-V2=(1?yu^%2EC7)SPaM>wws$Hzhm-;HPW?c}7^g&Z#3&m@8j zxpvZXO&R-8828qbxKd&op%+A2hMkc`agVH%lNGmmz%ac}=Iln=Fy!4Z6ly2nS_^HX z%CK;SS2i}tt4j+E>BL5?Q8!J~`(VXl{45Bi=%2wd#T7yiU=a_@)i!>B)mGP+tY5Id z6T@(w#tcQEjvF-2TfpZcf$?YICRWd-HYvv9i?yGat(}3LVg3tj>np6OZ^WlA2yWvR uF=?bqFBxtpi+5-`LTk92F!ykuqD75graWVT5*|`4ogYLH71Z!3ll>1t*aZ&& literal 0 HcmV?d00001 diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class new file mode 100644 index 0000000000000000000000000000000000000000..8fb20883b445ea631e179a6cea4b54657badaaf9 GIT binary patch literal 5547 zcmbVQd303O8UNkMByaL~Az>hDOhN`E7$zYjE-H?>l0SFOdafAr|F|Mm3Lp1RQAeQ#EX3xvbH_kH)? z?|#egTau&i9exJD3jAG#2PF!;8cI;A9nNh#Ah|A0xDQlaC#kiPEwr4ev=FNEjR1}|=GXa6p4%%<2v;ZO7amq%_ z^HQ-4mnc}SF~U{~%qY+;VH-n6l0X~NV`*dkpuic$unoH>I;LS2E>*Bv!^f~jpdv30 z{mk$gG8jbj3jys$R!3nBWJn4w6R2}I=-cg(8P|u5(8~1Sppi79VfiJM@7B;GH7`pi zr1yPTFEDFRhIk;NClZz&7%&2fbSx%CyF$YT^a@NPENtCOq}CccPYAHAJe%eDD>ZCF zALSlRQ|^nWkh>r>vo}vBNml(DHcOL~C5^Z>W=PVHj1zgE_=re+L$Va@0yCQ~ zD+GolQX>zN8d9J~rV&pJ#to9ElblzTE$?UA<&TOT*s0)J$@;pYT)xUm55$-&tD9C8 zGIz;j|0&#{;6@Ff#%GG|xv7~R9JG>gov$w`#6u==Z#mifg^y%TmE&``Nx|ned;vEX zKe97r8*%DlNyJsV+)OKUIm?30+lC(R@ZpOBXLWXV2J&VPY&24KfHYObZrrNiOB%i` ztH(^oro|i>$fJ^9EaAgf1?s!V);6LH7#79Ab{41rD@s6mGH{VJ$ZgV!U;psP&8X+6 zi|&w>@6HphWcH`9h>9@omL=yKG;mWm>{Me9z9|FnUY6UYuxpC<$pjq>Hu>-^YP%vD zr6O+pH%8k7jVZ?<4`}!{_K>xsH}luhQ}SK0`RITL=^J%LBx0meEZ3R$&TDOLmFWDg z%zk?X=HznZ`$_G?_XTRyu3;lGBKa6&W<)x|kw4Y$M+qL5cHAe?x_Jt-`QgRGewyuJ zA0B5`RwXJND0Erz(U@Ty5)&*rz2;Ctx6?`Dex+;4Z1r7QfY@vr|6p4$>tGt_vZCZw z*=;6_b?Nwkk=&>|QB}bT7#U%2(^&)uzl3 zmHFsul|jUGvyMU5V@Pc&N7YKpvQunJMtcl<*ovln__=_L9NCKn;%NpXYtN{aGHr9r zXbB8ic6&hhaFnMU4~G*`V@F|`Vjn4wI;UO~@Cen1mjwKdB!Tk+qe&yhPRQB+b&kHM zi2mCgy{&*=S%%-Mcp0xK_=7~$s{-@#+15;qStCZsHBpz-rmof_wv}Xcyt^<`G6!9S z*XW68%t+1awycr#Xh#8#GPW+D6kXi7bJUpPc%e(CD5e<}U3x6m%MOqr)WV5`k?e}; zsgyy7l;_$af+SnJ6#QAB@g!BJ(m=sq1!jNnQKk@AUCd#m`&ej7;g?-FSANpwLf{Fm zJ$RDe>`I+A+e&9<56QWLXZQ;S#~60jpJ<0W~aEOX&j zD9+A`-~^P~`!M4OwEo(|m<~+9KN+k&fa>58)bs~y{k7f+)Ez(rzt8EPK;r?-50)Q5 zP!6rGobsvpDCc+)lSvzOT8uNW1hwcuGt+(>R&v#a5I#<&Tz!OsaVo=_lTqm5=qiSZ4?oRQI^q@1|-R`EJI+jBeMHx71C# zCEk~t=m^5CmE>}qlHXdG0!GS!KOyVEu`E)xt8;MJe3u2i!0z$ zAM$GJDZlzGUwxjhzQ9+X=c_O9)uSi8%C0`6>-~hL1n26Ua*nR$x8H*q)7RvP=DCk? z-MWiIaFrTRh~OWOnJTiGM!aBE9vgmgNXI^mvI$z zR!VRjpPYsC$U$uHJ&5ZwXqUche*9$Qt!F~ZI(Dv8i~d11UT1E2gURuAtjC*7f3Ne@ zn;67D9r1k-Z^3bKIVPEY9bkz!7`9B@V%C%RtpApA>^cHZzmL_ZTN1uP0_Wy~_}XCx z7V*2Qq_eI{*@Fv@AlzRSt%|bD+_kQ`Zs9oYkvHxub#$+Db#Cq*$NhabdBoo1FUsBT zkU?|FI3DUbfoqkrtiDR!gSJAp?cV148gDV<`Xa_9WPDUI{$55<$vd084&w)o#PTU} zDWE@+qgRy`Z9JOY@IOX|`6{`MbsqRPE&U(Nz*~gze_7$*=3j>1VS#yMOooWIAY*w637Vme#UN*oha_?MU=riq#SH_P$QHmL6+C~NEg z(Dp7GSe>}Km`&^M<9wWd^!zb5NtMHH@qTn7bjtG#sh>KS`5B(&?4|!}nCE2&P{tq- v3`{w`;Eca;#$P()ublBW_#@BG<-Hot@~55s%Q1Ej5zeo1{uBO!zoF*;Oy!N_ literal 0 HcmV?d00001 diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java index 111e35f6..6e8fe35c 100644 --- a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java +++ b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java @@ -112,10 +112,9 @@ public void put(int frameIndex, BufferedImage frame) { throw new IllegalArgumentException("frame cannot be null"); } - totalLoads++; - lock.writeLock().lock(); try { + totalLoads++; // Only update if not already present (hit means no load needed) if (!cache.containsKey(frameIndex)) { cache.put(frameIndex, frame); @@ -181,7 +180,12 @@ public int getMaxSize() { * @return total loads count */ public long getTotalLoads() { - return totalLoads; + lock.readLock().lock(); + try { + return totalLoads; + } finally { + lock.readLock().unlock(); + } } /** @@ -190,7 +194,12 @@ public long getTotalLoads() { * @return total hits count */ public long getTotalHits() { - return totalHits; + lock.readLock().lock(); + try { + return totalHits; + } finally { + lock.readLock().unlock(); + } } /** @@ -199,10 +208,15 @@ public long getTotalHits() { * @return hit ratio as a double between 0 and 1, or 0.0 if no loads have occurred */ public double getHitRatio() { - if (totalLoads == 0) { - return 0.0; + lock.readLock().lock(); + try { + if (totalLoads == 0) { + return 0.0; + } + return (double) totalHits / totalLoads; + } finally { + lock.readLock().unlock(); } - return (double) totalHits / totalLoads; } /** @@ -212,8 +226,13 @@ public double getHitRatio() { */ @Override public String toString() { - return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", - size(), maxSize, totalHits, totalLoads, getHitRatio() * 100); + lock.readLock().lock(); + try { + return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", + cache.size(), maxSize, totalHits, totalLoads, getHitRatio() * 100); + } finally { + lock.readLock().unlock(); + } } /** From 01cb55361c807433209ac53a2189ab0576e2497e Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 20:22:11 +0000 Subject: [PATCH 2/6] correctness: Add capacity and eviction bounds check to FrameCache * Fix deadlock potential in FrameCache.toString() * Remove classes files and added .class to .gitignore * Change FrameCache.get() to use read lock instead of write lock Co-authored-by: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> --- .gitignore | 1 + .../cabrillo/tracker/video/FrameCache$1.class | Bin 1030 -> 0 bytes .../cabrillo/tracker/video/FrameCache.class | Bin 5547 -> 0 bytes .../cabrillo/tracker/video/FrameCache.java | 12 ++++++++---- 4 files changed, 9 insertions(+), 4 deletions(-) delete mode 100644 src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class delete mode 100644 src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class diff --git a/.gitignore b/.gitignore index 48f18a5b..88795848 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ src/org/opensourcephysics/controls/OSPLog_template.txt /target/ tracker.prefs //.classpath +*.class diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache$1.class deleted file mode 100644 index 9ea2b461de0756519bee9325d52f267df97e19f5..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1030 zcmbVLT~8B16g^Y6U0Mn)1q2lpwP;(^)hA*~@Po8rV<9FrJn%N`4(-Txx0#t%@yQ=! z;)^dPsEH5$0DqM6?v|vG@YGFq?wK>0Gxwgk^Y`EHKLB3gaTX~|*|73RBg61KQXMDi zi%>@c<%xdxR7+1g9zRkt2qMQQ?wttb45cq3r>S^P>~OCu%Fh@wrYm*%DT%ZuRH^4M zjjWAa9y7=@EWhVN?hK3!oR$nvgulymcaQh!tSuQzvrVAlHV>-@IVxi=j{=GemX@Cc z!&If}W>La|jm12caE)Pe!n_#l9{<=*WSPtGqB3!XNw$>ZPUKUL1u9GI4|+#Jz2ip# zF~vP$x{?2u$JQF5G{d|Sy=W*JfiJXaghrh*tW;WKZ^)5y!q=;Z46|)RcSPK|Vzw>Tj;xwl&fUc-V2=(1?yu^%2EC7)SPaM>wws$Hzhm-;HPW?c}7^g&Z#3&m@8j zxpvZXO&R-8828qbxKd&op%+A2hMkc`agVH%lNGmmz%ac}=Iln=Fy!4Z6ly2nS_^HX z%CK;SS2i}tt4j+E>BL5?Q8!J~`(VXl{45Bi=%2wd#T7yiU=a_@)i!>B)mGP+tY5Id z6T@(w#tcQEjvF-2TfpZcf$?YICRWd-HYvv9i?yGat(}3LVg3tj>np6OZ^WlA2yWvR uF=?bqFBxtpi+5-`LTk92F!ykuqD75graWVT5*|`4ogYLH71Z!3ll>1t*aZ&& diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.class deleted file mode 100644 index 8fb20883b445ea631e179a6cea4b54657badaaf9..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 5547 zcmbVQd303O8UNkMByaL~Az>hDOhN`E7$zYjE-H?>l0SFOdafAr|F|Mm3Lp1RQAeQ#EX3xvbH_kH)? z?|#egTau&i9exJD3jAG#2PF!;8cI;A9nNh#Ah|A0xDQlaC#kiPEwr4ev=FNEjR1}|=GXa6p4%%<2v;ZO7amq%_ z^HQ-4mnc}SF~U{~%qY+;VH-n6l0X~NV`*dkpuic$unoH>I;LS2E>*Bv!^f~jpdv30 z{mk$gG8jbj3jys$R!3nBWJn4w6R2}I=-cg(8P|u5(8~1Sppi79VfiJM@7B;GH7`pi zr1yPTFEDFRhIk;NClZz&7%&2fbSx%CyF$YT^a@NPENtCOq}CccPYAHAJe%eDD>ZCF zALSlRQ|^nWkh>r>vo}vBNml(DHcOL~C5^Z>W=PVHj1zgE_=re+L$Va@0yCQ~ zD+GolQX>zN8d9J~rV&pJ#to9ElblzTE$?UA<&TOT*s0)J$@;pYT)xUm55$-&tD9C8 zGIz;j|0&#{;6@Ff#%GG|xv7~R9JG>gov$w`#6u==Z#mifg^y%TmE&``Nx|ned;vEX zKe97r8*%DlNyJsV+)OKUIm?30+lC(R@ZpOBXLWXV2J&VPY&24KfHYObZrrNiOB%i` ztH(^oro|i>$fJ^9EaAgf1?s!V);6LH7#79Ab{41rD@s6mGH{VJ$ZgV!U;psP&8X+6 zi|&w>@6HphWcH`9h>9@omL=yKG;mWm>{Me9z9|FnUY6UYuxpC<$pjq>Hu>-^YP%vD zr6O+pH%8k7jVZ?<4`}!{_K>xsH}luhQ}SK0`RITL=^J%LBx0meEZ3R$&TDOLmFWDg z%zk?X=HznZ`$_G?_XTRyu3;lGBKa6&W<)x|kw4Y$M+qL5cHAe?x_Jt-`QgRGewyuJ zA0B5`RwXJND0Erz(U@Ty5)&*rz2;Ctx6?`Dex+;4Z1r7QfY@vr|6p4$>tGt_vZCZw z*=;6_b?Nwkk=&>|QB}bT7#U%2(^&)uzl3 zmHFsul|jUGvyMU5V@Pc&N7YKpvQunJMtcl<*ovln__=_L9NCKn;%NpXYtN{aGHr9r zXbB8ic6&hhaFnMU4~G*`V@F|`Vjn4wI;UO~@Cen1mjwKdB!Tk+qe&yhPRQB+b&kHM zi2mCgy{&*=S%%-Mcp0xK_=7~$s{-@#+15;qStCZsHBpz-rmof_wv}Xcyt^<`G6!9S z*XW68%t+1awycr#Xh#8#GPW+D6kXi7bJUpPc%e(CD5e<}U3x6m%MOqr)WV5`k?e}; zsgyy7l;_$af+SnJ6#QAB@g!BJ(m=sq1!jNnQKk@AUCd#m`&ej7;g?-FSANpwLf{Fm zJ$RDe>`I+A+e&9<56QWLXZQ;S#~60jpJ<0W~aEOX&j zD9+A`-~^P~`!M4OwEo(|m<~+9KN+k&fa>58)bs~y{k7f+)Ez(rzt8EPK;r?-50)Q5 zP!6rGobsvpDCc+)lSvzOT8uNW1hwcuGt+(>R&v#a5I#<&Tz!OsaVo=_lTqm5=qiSZ4?oRQI^q@1|-R`EJI+jBeMHx71C# zCEk~t=m^5CmE>}qlHXdG0!GS!KOyVEu`E)xt8;MJe3u2i!0z$ zAM$GJDZlzGUwxjhzQ9+X=c_O9)uSi8%C0`6>-~hL1n26Ua*nR$x8H*q)7RvP=DCk? z-MWiIaFrTRh~OWOnJTiGM!aBE9vgmgNXI^mvI$z zR!VRjpPYsC$U$uHJ&5ZwXqUche*9$Qt!F~ZI(Dv8i~d11UT1E2gURuAtjC*7f3Ne@ zn;67D9r1k-Z^3bKIVPEY9bkz!7`9B@V%C%RtpApA>^cHZzmL_ZTN1uP0_Wy~_}XCx z7V*2Qq_eI{*@Fv@AlzRSt%|bD+_kQ`Zs9oYkvHxub#$+Db#Cq*$NhabdBoo1FUsBT zkU?|FI3DUbfoqkrtiDR!gSJAp?cV148gDV<`Xa_9WPDUI{$55<$vd084&w)o#PTU} zDWE@+qgRy`Z9JOY@IOX|`6{`MbsqRPE&U(Nz*~gze_7$*=3j>1VS#yMOooWIAY*w637Vme#UN*oha_?MU=riq#SH_P$QHmL6+C~NEg z(Dp7GSe>}Km`&^M<9wWd^!zb5NtMHH@qTn7bjtG#sh>KS`5B(&?4|!}nCE2&P{tq- v3`{w`;Eca;#$P()ublBW_#@BG<-Hot@~55s%Q1Ej5zeo1{uBO!zoF*;Oy!N_ diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java index 6e8fe35c..6df4cf94 100644 --- a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java +++ b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java @@ -74,7 +74,7 @@ protected boolean removeEldestEntry(Map.Entry eldest) { * This method does not load a new frame - call loadFrame() for that.

* * @param frameIndex the frame index to retrieve - * @return the cached Bufferedlmage, or null if not found or index is invalid + * @return the cached BufferedImage, or null if not found * @throws IllegalStateException if frameIndex is invalid */ public BufferedImage get(int frameIndex) { @@ -82,15 +82,18 @@ public BufferedImage get(int frameIndex) { throw new IllegalStateException("frameIndex cannot be negative: " + frameIndex); } - lock.writeLock().lock(); + lock.readLock().lock(); try { BufferedImage frame = cache.get(frameIndex); if (frame != null) { + // Technically totalHits++ is a race condition here under read lock, + // but for a statistics counter it's an acceptable benign race + // compared to serializing all cache gets behind a write lock. totalHits++; } return frame; } finally { - lock.writeLock().unlock(); + lock.readLock().unlock(); } } @@ -228,8 +231,9 @@ public double getHitRatio() { public String toString() { lock.readLock().lock(); try { + double ratio = totalLoads == 0 ? 0.0 : (double) totalHits / totalLoads; return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", - cache.size(), maxSize, totalHits, totalLoads, getHitRatio() * 100); + cache.size(), maxSize, totalHits, totalLoads, ratio * 100); } finally { lock.readLock().unlock(); } From 21750b04ca5278b81228585ea6b736cdd34813d6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 20:27:06 +0000 Subject: [PATCH 3/6] correctness: Add capacity and eviction bounds check to FrameCache * Use LongAdder for totalLoads and totalHits for thread safety * Address PR feedback to fix reentrant deadlock in toString() and size() calls * Fix put() bug where totalLoads incremented even when frame already cached Co-authored-by: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> --- .../cabrillo/tracker/video/FrameCache.java | 54 ++++++------------- 1 file changed, 17 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java index 6df4cf94..b59a30b9 100644 --- a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java +++ b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java @@ -30,10 +30,10 @@ public class FrameCache { private final ReentrantReadWriteLock lock; /** Total frames loaded across all cache accesses */ - private long totalLoads; + private final java.util.concurrent.atomic.LongAdder totalLoads; /** Total frame cache hits */ - private long totalHits; + private final java.util.concurrent.atomic.LongAdder totalHits; /** * Creates a FrameCache with default maximum capacity of 100 frames. @@ -63,8 +63,8 @@ protected boolean removeEldestEntry(Map.Entry eldest) { } }; this.lock = new ReentrantReadWriteLock(); - this.totalLoads = 0; - this.totalHits = 0; + this.totalLoads = new java.util.concurrent.atomic.LongAdder(); + this.totalHits = new java.util.concurrent.atomic.LongAdder(); } /** @@ -86,10 +86,7 @@ public BufferedImage get(int frameIndex) { try { BufferedImage frame = cache.get(frameIndex); if (frame != null) { - // Technically totalHits++ is a race condition here under read lock, - // but for a statistics counter it's an acceptable benign race - // compared to serializing all cache gets behind a write lock. - totalHits++; + totalHits.increment(); } return frame; } finally { @@ -117,9 +114,9 @@ public void put(int frameIndex, BufferedImage frame) { lock.writeLock().lock(); try { - totalLoads++; // Only update if not already present (hit means no load needed) if (!cache.containsKey(frameIndex)) { + totalLoads.increment(); cache.put(frameIndex, frame); } } finally { @@ -183,12 +180,7 @@ public int getMaxSize() { * @return total loads count */ public long getTotalLoads() { - lock.readLock().lock(); - try { - return totalLoads; - } finally { - lock.readLock().unlock(); - } + return totalLoads.sum(); } /** @@ -197,12 +189,7 @@ public long getTotalLoads() { * @return total hits count */ public long getTotalHits() { - lock.readLock().lock(); - try { - return totalHits; - } finally { - lock.readLock().unlock(); - } + return totalHits.sum(); } /** @@ -211,15 +198,11 @@ public long getTotalHits() { * @return hit ratio as a double between 0 and 1, or 0.0 if no loads have occurred */ public double getHitRatio() { - lock.readLock().lock(); - try { - if (totalLoads == 0) { - return 0.0; - } - return (double) totalHits / totalLoads; - } finally { - lock.readLock().unlock(); + long loads = totalLoads.sum(); + if (loads == 0) { + return 0.0; } + return (double) totalHits.sum() / loads; } /** @@ -229,14 +212,11 @@ public double getHitRatio() { */ @Override public String toString() { - lock.readLock().lock(); - try { - double ratio = totalLoads == 0 ? 0.0 : (double) totalHits / totalLoads; - return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", - cache.size(), maxSize, totalHits, totalLoads, ratio * 100); - } finally { - lock.readLock().unlock(); - } + long loads = totalLoads.sum(); + long hits = totalHits.sum(); + double ratio = loads == 0 ? 0.0 : (double) hits / loads; + return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", + size(), maxSize, hits, loads, ratio * 100); } /** From 457d773cca188f1f5a396a18365487d5706cb8e4 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 20:31:59 +0000 Subject: [PATCH 4/6] correctness: Add capacity and eviction bounds check to FrameCache * Use LongAdder for totalLoads and totalHits for thread safety * Address PR feedback to fix reentrant deadlock in toString() and size() calls * Fix put() bug where totalLoads incremented even when frame already cached Co-authored-by: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> --- .gitignore | 1 - .../cabrillo/tracker/video/FrameCache.java | 19 ++++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 88795848..48f18a5b 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,3 @@ src/org/opensourcephysics/controls/OSPLog_template.txt /target/ tracker.prefs //.classpath -*.class diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java index b59a30b9..3ef28bb3 100644 --- a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java +++ b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java @@ -75,11 +75,11 @@ protected boolean removeEldestEntry(Map.Entry eldest) { * * @param frameIndex the frame index to retrieve * @return the cached BufferedImage, or null if not found - * @throws IllegalStateException if frameIndex is invalid + * @throws IllegalArgumentException if frameIndex is invalid */ public BufferedImage get(int frameIndex) { if (frameIndex < 0) { - throw new IllegalStateException("frameIndex cannot be negative: " + frameIndex); + throw new IllegalArgumentException("frameIndex cannot be negative: " + frameIndex); } lock.readLock().lock(); @@ -212,11 +212,16 @@ public double getHitRatio() { */ @Override public String toString() { - long loads = totalLoads.sum(); - long hits = totalHits.sum(); - double ratio = loads == 0 ? 0.0 : (double) hits / loads; - return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", - size(), maxSize, hits, loads, ratio * 100); + lock.readLock().lock(); + try { + long loads = totalLoads.sum(); + long hits = totalHits.sum(); + double ratio = loads == 0 ? 0.0 : (double) hits / loads; + return String.format("FrameCache[size=%d/%d, hits=%d, loads=%d, ratio=%.2f%%]", + cache.size(), maxSize, hits, loads, ratio * 100); + } finally { + lock.readLock().unlock(); + } } /** From 37f35bcd6ae157fbf840430f7e77a1e70b1c99a9 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 22:10:36 +0000 Subject: [PATCH 5/6] correctness: Add capacity and eviction bounds check to FrameCache * Use LongAdder for totalLoads and totalHits for thread safety * Address PR feedback to fix reentrant deadlock in toString() and size() calls * Fix put() bug where totalLoads incremented even when frame already cached Co-authored-by: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> --- .../opensourcephysics/cabrillo/tracker/video/FrameCache.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java index 3ef28bb3..76844380 100644 --- a/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java +++ b/src/main/java/org/opensourcephysics/cabrillo/tracker/video/FrameCache.java @@ -75,11 +75,11 @@ protected boolean removeEldestEntry(Map.Entry eldest) { * * @param frameIndex the frame index to retrieve * @return the cached BufferedImage, or null if not found - * @throws IllegalArgumentException if frameIndex is invalid + * @throws IllegalStateException if frameIndex is invalid */ public BufferedImage get(int frameIndex) { if (frameIndex < 0) { - throw new IllegalArgumentException("frameIndex cannot be negative: " + frameIndex); + throw new IllegalStateException("frameIndex cannot be negative: " + frameIndex); } lock.readLock().lock(); From 567abd5095122e18de34f0b9043edf142939409e Mon Sep 17 00:00:00 2001 From: Unluckyathecking <111193970+Unluckyathecking@users.noreply.github.com> Date: Tue, 26 May 2026 14:16:36 +0100 Subject: [PATCH 6/6] ci: jumpstart kimi review (post Pro upgrade)