RFR: Invoke implDeregister() at wakeup()

Miao Zheng duke at openjdk.java.net
Sat Apr 23 12:30:34 UTC 2022


On Fri, 22 Apr 2022 13:57:28 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Just to ACK that I've seen the PR. I am curious if you are seeing an issue or not. It shouldn't be necessary to explicitly deregister and I need a bit of time to page in some of the details to see if there are any side effects (async close mostly).
>
>> @AlanBateman Thanks for your review. If we do not explicit deregister will result epoll monitor a closed fd. Because a closed fd can be reused, I have not construct a case that the number of fd monitor by epoll overflow.
> 
> Would it be possible to provide a summary of the scenario that you are concerned about? It's one shot so will be disarmed when polled. If the thread is interrupted or unparked for some other reason then it will deregister the file descriptor. If the socket is closed then it will be removed the polling mechanism (the dup2 trick isn't not used with virtual threads, in case that is what you mean).

@AlanBateman Thanks for your review. The scenario is:
1. A virtual thread invoke InputStream.read();
2. Virtual thread register a fd into polling mechanism and park itself;
3. Another thread invoke InputStream.close(), and it will invoke wakeup() finally;
4. the map will remove the closed fd at wakeup(), and unpark vt which is invoke InputStream.read();
5. the unparked vt will invoke deregister() and it will find "null" at map.remove();

The result is ,the fd which is registered at InputStream.read() will remove at the map which holds the fd and corresponding parked vt, but not Deregister at epoll, so it will still monitor by epoll.

As your say, there are there cases:
1. polled: automatically disarmed
2. the thread is interrupted or unparked for some other reason: remove the fd at map and invoke ImplDeregister() of Epoll
3. the socket is closed: only remove the fd at map

I think we should do the same operation of 2 and 3, and I agree that we do not explicitly invoke implDeregister is ok, so I close this PR.

-------------

PR: https://git.openjdk.java.net/loom/pull/128


More information about the loom-dev mailing list