KQueue crash due to close/cleanup sequencing

Motivation:
The kqueue documentation states that 'Calling close() on a file descriptor will remove any kevents that reference the descriptor.' [1], but doesn't mention if this cleanup will be done synchronously. Under some circumstances it has been observed that cleanup was not done immediately and when KQueueEventLoop attempted to access the channel associated with the event the JVM would crash, a ClassCastException, or generally undefined behavior would occur because of invalid pointer references.

[1] https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2

Modifications:
- AbstractKqueueChannel#doClose should not rely upon this assumption and instead should call doDeregister() to ensure cleanup is done synchronously.
- Deleting a kevent should also set the jniSelfPtr stored in the udata of that kevent to NULL, to ensure we will not dereference it later.

Result:
No more kqueue crash due to close/cleanup sequencing.
This commit is contained in:
Scott Mitchell 2017-06-28 14:03:47 -04:00
parent 8d0e0922a5
commit 1df8f2ccd1
2 changed files with 7 additions and 3 deletions

View File

@ -32,6 +32,9 @@ static void netty_kqueue_eventarray_evSet(JNIEnv* env, jclass clzz, jlong kevent
if (jniSelfPtr == 0) {
jniSelfPtr = (jlong) (*env)->NewGlobalRef(env, channel);
(*env)->SetLongField(env, channel, kqueueJniPtrFieldId, jniSelfPtr);
} else if ((flags & EV_DELETE) != 0) {
// If the event is deleted, make sure it no longer has a reference to the jniSelfPtr because it shouldn't be used after this point.
jniSelfPtr = 0;
}
EV_SET((struct kevent*) keventAddress, ident, filter, flags, fflags, 0, (jobject) jniSelfPtr);
}

View File

@ -98,11 +98,12 @@ abstract class AbstractKQueueChannel extends AbstractChannel implements UnixChan
// Even if we allow half closed sockets we should give up on reading. Otherwise we may allow a read attempt on a
// socket which has not even been connected yet. This has been observed to block during unit tests.
inputClosedSeenErrorOnRead = true;
// The FD will be closed, which will take of deleting from kqueue.
readFilterEnabled = writeFilterEnabled = false;
try {
if (isRegistered()) {
((KQueueEventLoop) eventLoop()).remove(this);
// The FD will be closed, which should take care of deleting any associated events from kqueue, but
// since we rely upon jniSelfRef to be consistent we make sure that we clear this reference out for all]
// events which are pending in kqueue to avoid referencing a deleted pointer at a later time.
doDeregister();
}
} finally {
socket.close();