concurrent modification of ChannelsLimiter.java

[buska88]

is synchronized (isPaused) necessary in handlerAdded and resumeAllChannels?can declare isPaused as volatile work?its value has been declared volatile, so i cannot understand why there will be a concurrent problem. has some channels been set autoread false online?in handlerAdded，channels.add(ctx.channel()); happens before isPaused.get();and in resumeAllChannels,isPaused.set(false) happens before channels.forEach()，so i think after isPaused.set(false),the channel has already been add to channels. May channels.forEach() excucutes before isPaused.set(false) in resumeAllChannels due to reorder?pauseAllChannels may enconunter the same problem.so why not add synchronized (isPaused) in pauseAllChannels in case of some channels be incorrectly set autoread true?

[buska88]

@FMX hope your attention about this question

[FMX]

@buska88 Hi, the conclusion is YES. You can refer to the commit "[BUG] After trigger pausePushData, new added channel still can push data (#376)"(git hash:342adea0).

[FMX]

isPaused is synced because channels might be modified when changing the flag isPaused. That will cause some channels to remain in the wrong auto-read state. The channel limiter needs to make sure that changing all channels' auto-read state and changing the isPause flag is an atomic operation.

[buska88]

thanks for replying.