On Tue, 30 Jan 2024 16:15:17 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>>> If we are going to check close for errors then we will have to know for >>> certain we are only trying to close open fd's, and we will have to deal >>> with EINTR. I would think this is a "best effort" to close open FD's and >>> not something we have to care about in detail. >> >> Right, EINTR would need to be ignored as it would be wrong to restart. I'm >> scratching my head as to why this code would even do with EIO or if it even >> possible here. > > Looking at man close, EIO gets returned if: >> [EIO] A previously-uncommitted write(2) encountered an >> input/output error. > > If I understand this `launch=<cmd>` option correctly for the jdwp agent, then > the arbitrary command gets launched very early in the lifecycle of the JVM. > So in theory, I think there shouldn't be too many file descriptors that the > JVM has opened so far and the chances of it having started writing to some of > those opened file descriptors (other than standard out/err) perhaps is slim? > But that's just a guess. Probably not a strong one too, given that if the JVM > was launched with any other agent along with jdwp, then it's unknown if the > other agent would have opened any file descriptors for write. > > My understanding of these close() calls after fork() in this code, matches > David's - I think it's a best effort attempt. So perhaps ignoring such > failures, like we do now, is OK? Ignoring errors just doesn't seem like a good strategy. This fix right here that we are doing doesn't have to deal with making this code more robust, as long as we follow up in another issue. For that, however, I filed JDK-8324979, and that frees us up to move on with this fix. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17588#discussion_r1471566159
