Issue 29567613: Issue 5572 - reduce the probability of the race condition while updating of preloaded subscriptions
|
Created:
Oct. 6, 2017, 9:06 a.m. by sergei Modified:
Nov. 17, 2017, 4:13 p.m. CC:
René Jeschke Base URL:
github.com:abby-sergz/libadblockplus-android.git Visibility:
Public. |
Patch Set 1 #
Total comments: 2
MessagesTotal messages: 6
https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... File libadblockplus-android/jni/JniFilterEngine.cpp (right): https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... libadblockplus-android/jni/JniFilterEngine.cpp:502: timer.SetTimer(std::chrono::milliseconds(300), updateSubscriptionFilters); I don't like such timers a lot. It's always introduces hard reproduceable cases and extremely hard to find and fix. Does this (https://issues.adblockplus.org/ticket/5572#comment:12) help?
https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... File libadblockplus-android/jni/JniFilterEngine.cpp (right): https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... libadblockplus-android/jni/JniFilterEngine.cpp:502: timer.SetTimer(std::chrono::milliseconds(300), updateSubscriptionFilters); On 2017/10/06 10:38:08, anton wrote: > I don't like such timers a lot. It's always introduces hard reproduceable cases > and extremely hard to find and fix. Does this > (https://issues.adblockplus.org/ticket/5572#comment:12) help? As discussed in the issue, it does not help.
On 2017/10/06 13:37:04, sergei wrote: > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > File libadblockplus-android/jni/JniFilterEngine.cpp (right): > > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > libadblockplus-android/jni/JniFilterEngine.cpp:502: > timer.SetTimer(std::chrono::milliseconds(300), updateSubscriptionFilters); > On 2017/10/06 10:38:08, anton wrote: > > I don't like such timers a lot. It's always introduces hard reproduceable > cases > > and extremely hard to find and fix. Does this > > (https://issues.adblockplus.org/ticket/5572#comment:12) help? > > As discussed in the issue, it does not help. Then i'd prefer it to be fixed in proper way, with queue or smth
On 2017/10/10 06:21:29, anton wrote: > On 2017/10/06 13:37:04, sergei wrote: > > > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > > File libadblockplus-android/jni/JniFilterEngine.cpp (right): > > > > > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > > libadblockplus-android/jni/JniFilterEngine.cpp:502: > > timer.SetTimer(std::chrono::milliseconds(300), updateSubscriptionFilters); > > On 2017/10/06 10:38:08, anton wrote: > > > I don't like such timers a lot. It's always introduces hard reproduceable > > cases > > > and extremely hard to find and fix. Does this > > > (https://issues.adblockplus.org/ticket/5572#comment:12) help? > > > > As discussed in the issue, it does not help. > > Then i'd prefer it to be fixed in proper way, with queue or smth any update on this? the bug still remains
On 2017/11/16 10:47:10, anton wrote: > On 2017/10/10 06:21:29, anton wrote: > > On 2017/10/06 13:37:04, sergei wrote: > > > > > > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > > > File libadblockplus-android/jni/JniFilterEngine.cpp (right): > > > > > > > > > https://codereview.adblockplus.org/29567613/diff/29567614/libadblockplus-andr... > > > libadblockplus-android/jni/JniFilterEngine.cpp:502: > > > timer.SetTimer(std::chrono::milliseconds(300), updateSubscriptionFilters); > > > On 2017/10/06 10:38:08, anton wrote: > > > > I don't like such timers a lot. It's always introduces hard reproduceable > > > cases > > > > and extremely hard to find and fix. Does this > > > > (https://issues.adblockplus.org/ticket/5572#comment:12) help? > > > > > > As discussed in the issue, it does not help. > > > > Then i'd prefer it to be fixed in proper way, with queue or smth > > any update on this? the bug still remains No updates. I would propose to land it but don't close the issue. After adding a support of it in the core this code will be removed. 
|
||||||||||||||||||||||||||||||||||||||||||
