[JENKINS-70206] Lazy initialization of JnlpSlaveAgentProtocol4

[jglick]

See JENKINS-70206. Supersedes #7512.

Currently just unconditionally returning a value from getName, and letting handle throw up any problems. We could try to disable the protocol (return null from getName) in case of problems, such as a missing instance-identity plugin (#6585), but that would be a bit more complicated, possibly cause extra work during startup even when this protocol is actually unused, and it is not clear to me that this would actually improve behavior for the user anyway.

Testing done

Just relying on automated tests here. Not sure if we can tell whether the otherwise flaky test is more reliable with this patch.

Proposed changelog entries

• Delaying initialization of cryptography needed for TCP inbound agents unless and until such an agent is connected.

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[jglick]

Arguably more legible just below handle but this keeps the diff smaller.

[jglick]

KeyStoreException, KeyManagementException but also IllegalStateException…just seemed simpler to catch anything.

[Vlatombe]

Extract to constant?

[jglick]

Could, though this would require a new remoting release.

[Vlatombe]

Ah right, missed the fact it was somewhere else.

[Vlatombe]

Suggested change

	try {
	init();
	} catch (IOException x) {
	throw x;
	} catch (Exception x) {
	throw new IOException(x);
	}
	if (handler == null) {
	try {
	init();
	} catch (IOException x) {
	throw x;
	} catch (Exception x) {
	throw new IOException(x);
	}
	}

to avoid taking the object monitor in the general case?

[jglick]

Would have to think about whether that requires making handler be volatile. Does not seem necessary since handle should be called once per actual agent connection, which is many orders of magnitude more expensive than acquiring an uncontended monitor.

[basil]

FTR this pattern would work, but as that page itself mentions, "in modern JVMs with efficient uncontended synchronization the performance difference is often negligible". The performance of the current version is certainly fine as described in the previous comment.

[basil]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!