Bug 65311 - NioBlockingSelector.BlockPoller.run() may not "wake up" in time
Summary: NioBlockingSelector.BlockPoller.run() may not "wake up" in time
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Connectors (show other bugs)
Version: 8.5.31
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-18 08:37 UTC by huangqiaohui
Modified: 2021-05-26 15:49 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description huangqiaohui 2021-05-18 08:37:00 UTC
org.apache.tomcat.util.net.NioBlockingSelector.BlockPoller: 

227  public void wakeup() {
228 		if (wakeupCounter.addAndGet(1)==0) selector.wakeup();
229  }
....
287  public void run() {
288  	while (run) {
289  		try {
290  			events();
291  			int keyCount = 0;
292  			try {
293  				int i = wakeupCounter.get();
294  				if (i>0)
295  					keyCount = selector.selectNow();
296  				else {
297  					wakeupCounter.set(-1);
298  					keyCount = selector.select(1000);
299  				}
300  				wakeupCounter.set(0);

when wakeupCounter is 0, selector.select(1000) may not be wakeup in time.
to resolve this bug, we can see org.apache.tomcat.util.net.NioEndpoint.Poller:

782  public void run() {
783  	// Loop until destroy() is called
784  	while (true) {
785  
786  		boolean hasEvents = false;
787  
788  		try {
789  			if (!close) {
790  				hasEvents = events();
791  				if (wakeupCounter.getAndSet(-1) > 0) {
792  					//if we are here, means we have other stuff to do
793  					//do a non blocking select
794  					keyCount = selector.selectNow();
795  				} else {
796  					keyCount = selector.select(selectorTimeout);
797  				}
798  				wakeupCounter.set(0);
799  			}
Comment 1 Mark Thomas 2021-05-18 09:59:41 UTC
Thanks for the report.

Fixed in:
- 9.0.x for 9.0.47 onwards
- 8.5.x for 8.5.67 onwards
Comment 2 Remy Maucherat 2021-05-18 14:53:38 UTC
Too bad I had to revert the NioBlockingSelector removal, I was too worried of the initial regression I never tried to backport it again. But nobody has complained about it in Tomcat 10, so it's likely less risky now.
Comment 3 Mark Thomas 2021-05-18 16:45:54 UTC
I'd have no objections if you wanted to back-port it now.
Comment 4 Remy Maucherat 2021-05-26 15:49:43 UTC
(In reply to Mark Thomas from comment #3)
> I'd have no objections if you wanted to back-port it now.

I did the backport it works fine., but I'm having second thoughts.

I think this might cause a performance degradation on Windows. The JDK 17 changelog with its NIO update for Windows clearly hints the selector is limited compared to Unix until Java 17 is out. In that case, having a dedicated poller for blocking IO could be better (two selectors being better than one if the selector scalability degrades with the socket count, and that's not counting the hack configs to have more more more more).

Or we can say anyone really affected can try NIO2.

What's the final call ?