rzo1 commented on code in PR #293:
URL: https://github.com/apache/openjpa/pull/293#discussion_r4522794585
##########
openjpa-kernel/src/main/java/org/apache/openjpa/event/TCPRemoteCommitProvider.java:
##########
@@ -282,10 +325,55 @@ public void setAddresses(final String names) throws
UnknownHostException {
}
}
} finally {
+ addressesChanged();
_addressesLock.unlock();
}
}
+ /**
+ * Updates the addresses from which incoming connections are accepted.
Must be invoked, while holding
+ * {@link #_addressesLock}, whenever {@link #_addresses} has been changed.
+ */
+ protected void addressesChanged() {
+ _peerAddresses =
_addresses.stream().map(HostAddress::getAddress).collect(Collectors.toUnmodifiableSet());
+ }
+
+ /**
+ * Whether this provider accepts an incoming connection from the given
address. Connections from this host are
+ * always accepted; connections from other hosts only if they are one of
the configured peers, unless
+ * {@link #setRestrictToPeers(boolean) RestrictToPeers} is turned off. The
port of the peers is not considered, as
+ * the peers connect from arbitrary local ports.
+ *
+ * @param address the address of the remote end of the incoming connection
+ * @return <code>true</code> if the connection is accepted
+ */
+ protected boolean acceptsConnectionFrom(final InetAddress address) {
+ return !_restrictToPeers || address.isLoopbackAddress() ||
localAddresses.contains(address)
Review Comment:
Yes, two cases: `localAddresses` holds the addresses of the network
interfaces, i.e. `127.0.0.1`/`::1`, while all of `127.0.0.0/8` is loopback, so
a peer on this host bound to e.g. `127.0.0.2` would not be in it. And if the
interfaces cannot be listed, `localAddresses` falls back to
`InetAddress.getLocalHost()` alone, without any loopback address. I added a
comment explaining this (5d52458a6).
##########
openjpa-kernel/src/main/java/org/apache/openjpa/event/DynamicTCPRemoteCommitProvider.java:
##########
@@ -51,13 +69,55 @@ public final void setAddresses(final String names) throws
UnknownHostException {
public void endConfiguration() {
TcpAddressesUpdater updater = new TcpAddressesUpdater();
updater.run();
+ _updater = updater;
Timer timer = new Timer(true);
timer.scheduleAtFixedRate(updater, 0, _cacheDurationMillis);
super.endConfiguration();
}
+ /**
+ * A peer that just came up may connect before the periodic refresh has
discovered it, so the addresses are
+ * refreshed before a connection from an unknown host is rejected.
+ */
+ @Override
+ protected boolean refreshesAddressesOnDemand() {
+ return isRestrictToPeers() && _updater != null;
+ }
+
+ /**
+ * Refreshes the addresses, unless a refresh started after the connection
was accepted. Refreshes run one at a
+ * time and at most once per second, so that connections from unknown
hosts cannot flood the source of the
+ * addresses.
+ */
+ @Override
+ protected void refreshAddresses(final long acceptedNanos) {
Review Comment:
It's about the clock rather than the precision: `System.nanoTime()` is
monotonic, `System.currentTimeMillis()` follows the wall clock and can jump
(NTP, manual changes). The value is compared with the time the connection was
accepted to decide whether a refresh already started after it, which a clock
jump could get wrong. The interval itself is defined as
`TimeUnit.SECONDS.toNanos(1)`. I documented this on the field (5d52458a6).
##########
openjpa-kernel/src/main/java/org/apache/openjpa/event/TCPRemoteCommitProvider.java:
##########
Review Comment:
Right, a new listener already gets the provider before `listen()`, so the
second call only re-added it to the set. It is now only called for an existing
listener (5d52458a6).
##########
openjpa-kernel/src/main/java/org/apache/openjpa/event/TCPRemoteCommitProvider.java:
##########
@@ -634,6 +733,76 @@ public void run() {
}
}
+ /**
+ * The outcome of the check of an incoming connection on the accept
thread.
+ */
+ private enum Admission {
+ /** The connection is closed. */
+ REJECTED,
+ /** The connection comes from this host or a known peer. */
+ PEER,
+ /**
+ * The connection comes from an unknown host; it is only received
if a refresh of the addresses finds
+ * the host, see {@link ReceiveSocketHandler#isPeerAfterRefresh()}.
+ */
+ UNKNOWN_HOST
+ }
+
+ private List<TCPRemoteCommitProvider> providers() {
+ synchronized (_providers) {
+ return new ArrayList<>(_providers);
Review Comment:
Done (5d52458a6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
