Skip to content

Commit 17cc2f6

Browse files
committed
Suggest some fine-tuning for foreign PR
In [PR_127] we got miscellaneous improvements. But we do no longer use poll and therefore are back to the FD_SETSIZE limit of *select*. This commit suggests some fine-tuning to [PR_127]. It does so by replacing a possible SEGFAULT (aka crashing the whole JVM) by throwing a java exception instead. This improves the reported error and even enables user code to react to the situation. Still not perfect. But IMHO good enough for now. We can still improve later (for example as soon there's enough time to do so). [PR_127]: java-native#127
1 parent 7da1377 commit 17cc2f6

1 file changed

Lines changed: 22 additions & 65 deletions

File tree

src/main/cpp/_nix_based/jssc.cpp

Lines changed: 22 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,9 @@
4040
#endif
4141
#ifdef __APPLE__
4242
#include <serial/ioss.h>//Needed for IOSSIOSPEED in Mac OS X (Non standard baudrate)
43-
#elif !defined(HAVE_POLL)
44-
// Seems as poll has some portability issues on OsX (Search for "poll" in
45-
// "https://cr.yp.to/docs/unixport.html"). So we only make use of poll on
46-
// all platforms except "__APPLE__".
47-
// If you want to force usage of 'poll', pass "-DHAVE_POLL=1" to gcc.
48-
#define HAVE_POLL 1
4943
#endif
5044

51-
#if HAVE_POLL == 0
52-
#include <sys/select.h>
53-
#else
54-
#include <poll.h>
55-
#endif
45+
#include <sys/select.h>
5646

5747
#include <jni.h>
5848
#include <jssc_SerialNativeInterface.h>
@@ -527,15 +517,23 @@ JNIEXPORT jboolean JNICALL Java_jssc_SerialNativeInterface_setDTR
527517
*/
528518
JNIEXPORT jboolean JNICALL Java_jssc_SerialNativeInterface_writeBytes
529519
(JNIEnv *env, jobject, jlong portHandle, jbyteArray buffer){
530-
jbyte* jBuffer = env->GetByteArrayElements(buffer, JNI_FALSE);
520+
jbyte* jBuffer = NULL;
531521
jint bufferSize = env->GetArrayLength(buffer);
532522
fd_set write_fd_set;
533523
int byteRemains = bufferSize;
534524
struct timeval timeout;
535525
int result;
536526
jclass threadClass = env->FindClass("java/lang/Thread");
537527
jmethodID areWeInterruptedMethod = env->GetStaticMethodID(threadClass, "interrupted", "()Z");
538-
528+
529+
if (portHandle < 0 || portHandle > FD_SETSIZE) {
530+
char msg[95];
531+
snprintf(msg, sizeof msg, "select(): file descriptor %ld outside expected range 0..%d", portHandle, FD_SETSIZE);
532+
return env->ThrowNew(env->FindClass("java/lang/IndexOutOfBoundsException"), msg);
533+
}
534+
535+
jBuffer = env->GetByteArrayElements(buffer, JNI_FALSE);
536+
539537
while(byteRemains > 0) {
540538

541539
// Check if the java thread has been interrupted, and if so, throw the exception
@@ -576,70 +574,29 @@ JNIEXPORT jboolean JNICALL Java_jssc_SerialNativeInterface_writeBytes
576574
return JNI_TRUE; //result == bufferSize ? JNI_TRUE : JNI_FALSE;
577575
}
578576

579-
/**
580-
* Waits until 'read()' has something to tell for the specified filedescriptor.
581-
*/
582-
/*
583-
static void awaitReadReady(JNIEnv*, jlong fd){
584-
#if HAVE_POLL == 0
585-
// Alternative impl using 'select' as 'poll' isn't available (or broken).
586-
587-
//assert(fd < FD_SETSIZE); // <- Might help when hunting SEGFAULTs.
588-
fd_set readFds;
589-
while(true) {
590-
FD_ZERO(&readFds);
591-
FD_SET(fd, &readFds);
592-
int result = select(fd + 1, &readFds, NULL, NULL, NULL);
593-
if(result < 0){
594-
// man select: On error, -1 is returned, and errno is set to indicate the error
595-
// TODO: Maybe a candidate to raise a java exception. But won't do
596-
// yet for backward compatibility.
597-
continue;
598-
}
599-
// Did wait successfully.
600-
break;
601-
}
602-
FD_CLR(fd, &readFds);
603-
604-
#else
605-
// Default impl using 'poll'. This is more robust against fd>=1024 (eg
606-
// SEGFAULT problems).
607-
608-
struct pollfd fds[1];
609-
fds[0].fd = fd;
610-
fds[0].events = POLLIN;
611-
while(true){
612-
int result = poll(fds, 1, -1);
613-
if(result < 0){
614-
// man poll: On error, -1 is returned, and errno is set to indicate the error.
615-
// TODO: Maybe a candidate to raise a java exception. But won't do
616-
// yet for backward compatibility.
617-
continue;
618-
}
619-
// Did wait successfully.
620-
break;
621-
}
622577

623-
#endif
624-
}
625-
*/
626-
627-
/* OK */
628578
/*
629579
* Reading data from the port
630-
*
631-
* Rewritten to use poll() instead of select() to handle fd>=1024
632580
*/
633581
JNIEXPORT jbyteArray JNICALL Java_jssc_SerialNativeInterface_readBytes
634582
(JNIEnv *env, jobject, jlong portHandle, jint byteCount){
635583
fd_set read_fd_set;
636-
jbyte *lpBuffer = new jbyte[byteCount];
584+
jbyte *lpBuffer = NULL;
637585
int byteRemains = byteCount;
638586
struct timeval timeout;
639587
int result;
640588
jclass threadClass = env->FindClass("java/lang/Thread");
641589
jmethodID areWeInterruptedMethod = env->GetStaticMethodID(threadClass, "interrupted", "()Z");
642-
590+
591+
if (portHandle < 0 || portHandle > FD_SETSIZE) {
592+
char msg[95];
593+
snprintf(msg, sizeof msg, "select(): file descriptor %ld outside expected range 0..%d", portHandle, FD_SETSIZE);
594+
env->ThrowNew(env->FindClass("java/lang/IndexOutOfBoundsException"), msg);
595+
return NULL;
596+
}
597+
598+
lpBuffer = new jbyte[byteCount];
599+
643600
while(byteRemains > 0) {
644601

645602
// Check if the java thread has been interrupted, and if so, throw the exception

0 commit comments

Comments
 (0)