> > The rule of the thumb is: When the problem shouldn't happen - i.e.
> > there's a bug in 0MQ of Java binding - assert. If the problem can be
> > caused by the user raise an exception.
>
> Ok, if that is the rule there are several assert() calls that will
stay
> the same; perhaps all of them. I will report back.
Find attached a patch that does the following:
* Move the raise_exception() function to a separate util.{hpp,cpp} file;
it is used from Socket.cpp and Poller.cpp.
* Use assert() only when signaling an error that the user cannot do
anything about; in all other cases, raise an exception.
* Check the error code returned by all calls to 0MQ functions (there
were a few missing). When checking this, always do (rc != 0) instead of
(rc == -1).
* After a call to a 0MQ function, assign errno to err; this is to ensure
errno is not overwritten after the 0MQ call but before raising an
exception that reports that error.
* Make sure multiple initialization / finalizations of Context and
Socket have no effect, to allow explicit calls to term() / close().
I know I have write access to the Java binding repo, but I prefer to
show my changes before committing them. Any comments before I do?
--
Gonzalo Diethelm
-----------------------------------------
Declaración de confidencialidad: Este Mensaje esta destinado para
el uso de la o las personas o entidades a quien ha sido dirigido y
puede contener información reservada y confidencial que no puede
ser divulgada, difundida, ni aprovechada en forma alguna. El uso no
autorizado de la información contenida en este correo podrá ser
sancionado de conformidad con la ley chilena.
Si usted ha recibido este correo electrónico por error, le pedimos
eliminarlo junto con los archivos adjuntos y avisar inmediatamente
al remitente, respondiendo este mensaje.
"Before printing this e-mail think if is really necesary".
Disclosure: This Message is to be used by the individual,
individuals or entities that it is addressed to and may include
private and confidential information that may not be disclosed,
made public nor used in any way at all. Unauthorized use of the
information in this electronic mail message may be subject to the
penalties set forth by Chilean law.
If you have received this electronic mail message in error, we ask
you to destroy the message and its attached file(s) and to
immediately notify the sender by answering this message.
diff --git a/src/Context.cpp b/src/Context.cpp
index bcfca5a..f901d52 100755
--- a/src/Context.cpp
+++ b/src/Context.cpp
@@ -22,7 +22,8 @@
#include <zmq.h>
-#include "config.hpp"
+#include "jzmq.hpp"
+#include "util.hpp"
#include "org_zmq_Context.h"
/** Handle to Java's Context::contextHandle. */
@@ -45,10 +46,14 @@ static void ensure_context (JNIEnv *env, jobject obj)
/**
* Get the value of Java's Context::contextHandle.
*/
-static void *get_context (JNIEnv *env, jobject obj)
+static void *get_context (JNIEnv *env, jobject obj, int do_assert)
{
ensure_context (env, obj);
void *s = (void*) env->GetLongField (obj, ctx_handle_fid);
+
+ if (do_assert)
+ assert (s);
+
return s;
}
@@ -62,38 +67,21 @@ static void put_context (JNIEnv *env, jobject obj, void *s)
}
/**
- * Raise an exception that includes 0MQ's error message.
- */
-static void raise_exception (JNIEnv *env, int err)
-{
- // Get exception class.
- jclass exception_class = env->FindClass ("java/lang/Exception");
- assert (exception_class);
-
- // Get text description of the exception.
- const char *err_msg = zmq_strerror (err);
-
- // Raise the exception.
- int rc = env->ThrowNew (exception_class, err_msg);
- env->DeleteLocalRef (exception_class);
-
- assert (rc == 0);
-}
-
-/**
* Called to construct a Java Context object.
*/
JNIEXPORT void JNICALL Java_org_zmq_Context_construct (JNIEnv *env,
jobject obj, jint app_threads, jint io_threads, jint flags)
{
- void *c = get_context (env, obj);
- assert (!c);
+ void *c = get_context (env, obj, 0);
+ if (c)
+ return;
c = zmq_init (app_threads, io_threads, flags);
+ int err = errno;
put_context (env, obj, c);
- if (!c) {
- raise_exception (env, errno);
+ if (c == NULL) {
+ raise_exception (env, err);
return;
}
}
@@ -104,10 +92,17 @@ JNIEXPORT void JNICALL Java_org_zmq_Context_construct
(JNIEnv *env,
JNIEXPORT void JNICALL Java_org_zmq_Context_finalize (JNIEnv *env,
jobject obj)
{
- void *c = get_context (env, obj);
- assert (c);
+ void *c = get_context (env, obj, 0);
+ if (! c)
+ return;
int rc = zmq_term (c);
- put_context (env, obj, NULL);
- assert (rc == 0);
+ int err = errno;
+ c = NULL;
+ put_context (env, obj, c);
+
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
}
diff --git a/src/Makefile.am b/src/Makefile.am
index 8b54d50..1bf8b03 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,17 +13,19 @@ jar_DATA = $(jarfile)
dist_noinst_JAVA = \
org/zmq/Context.java \
org/zmq/Socket.java \
- org/zmq/Poller.java
+ org/zmq/Poller.java \
lib_LTLIBRARIES = libjzmq.la
libjzmq_la_SOURCES = \
Context.cpp \
Socket.cpp \
- Poller.cpp
+ Poller.cpp \
+ util.cpp \
+
nodist_libjzmq_la_SOURCES = \
org_zmq_Context.h \
org_zmq_Socket.h \
- org_zmq_Poller.h
+ org_zmq_Poller.h \
libjzmq_la_CXXFLAGS = -Wall
libjzmq_la_LDFLAGS = -version-info @JLTVER@
@@ -34,7 +36,7 @@ BUILT_SOURCES = \
org/zmq/Socket.class \
org_zmq_Socket.h \
org/zmq/Poller.class \
- org_zmq_Poller.h
+ org/zmq/Poller.h \
CLEANFILES = \
org/zmq/Context.class \
@@ -43,7 +45,7 @@ CLEANFILES = \
org_zmq_Socket.h \
org/zmq/Poller.class \
org_zmq_Poller.h \
- Zmq.jar
+ zmq.jar \
$(srcdir)/Context.cpp: org_zmq_Context.h
@@ -66,6 +68,8 @@ org_zmq_Poller.h: org/zmq/Poller.class
./org/zmq/Poller.class: classdist_noinst.stamp
+$(srcdir)/util.cpp: util.h
+
dist-hook:
-rm $(distdir)/*.h
diff --git a/src/Poller.cpp b/src/Poller.cpp
index 8a42c11..d55db07 100755
--- a/src/Poller.cpp
+++ b/src/Poller.cpp
@@ -23,6 +23,7 @@
#include <zmq.h>
#include "jzmq.hpp"
+#include "util.hpp"
#include "org_zmq_Poller.h"
static void *fetch_socket (JNIEnv *env, jobject socket);
@@ -66,8 +67,10 @@ JNIEXPORT jlong JNICALL Java_org_zmq_Poller_run_1poll
(JNIEnv *env,
if (!s_0mq)
continue;
void *s = fetch_socket (env, s_0mq);
- if (!s)
+ if (s == NULL) {
+ raise_exception (env, EINVAL);
continue;
+ }
pitem [pc].socket = s;
pitem [pc].fd = 0;
pitem [pc].events = e_0mq [i];
@@ -102,7 +105,6 @@ JNIEXPORT jlong JNICALL Java_org_zmq_Poller_run_1poll
(JNIEnv *env,
/**
* Get the value of socketHandle for the specified Java Socket.
- * TODO: move this to a single util.h file.
*/
static void *fetch_socket (JNIEnv *env, jobject socket)
{
@@ -122,6 +124,5 @@ static void *fetch_socket (JNIEnv *env, jobject socket)
s = NULL;
}
- assert (s);
return s;
}
diff --git a/src/Socket.cpp b/src/Socket.cpp
index ec84111..edf8d2c 100755
--- a/src/Socket.cpp
+++ b/src/Socket.cpp
@@ -24,6 +24,7 @@
#include <zmq.h>
#include "jzmq.hpp"
+#include "util.hpp"
#include "org_zmq_Socket.h"
/** Handle to Java's Socket::socketHandle. */
@@ -46,10 +47,14 @@ static void ensure_socket (JNIEnv *env, jobject obj)
/**
* Get the value of Java's Socket::socketHandle.
*/
-static void *get_socket (JNIEnv *env, jobject obj)
+static void *get_socket (JNIEnv *env, jobject obj, int do_assert)
{
ensure_socket (env, obj);
void *s = (void*) env->GetLongField (obj, socket_handle_fid);
+
+ if (do_assert)
+ assert (s);
+
return s;
}
@@ -69,7 +74,7 @@ static void *fetch_context (JNIEnv *env, jobject context)
{
static jmethodID get_context_handle_mid = NULL;
- if (!get_context_handle_mid) {
+ if (get_context_handle_mid == NULL) {
jclass cls = env->GetObjectClass (context);
assert (cls);
get_context_handle_mid = env->GetMethodID (cls,
@@ -83,44 +88,31 @@ static void *fetch_context (JNIEnv *env, jobject context)
c = NULL;
}
- assert (c);
return c;
}
/**
- * Raise an exception that includes 0MQ's error message.
- */
-static void raise_exception (JNIEnv *env, int err)
-{
- // Get exception class.
- jclass exception_class = env->FindClass ("java/lang/Exception");
- assert (exception_class);
-
- // Get text description of the exception.
- const char *err_msg = zmq_strerror (err);
-
- // Raise the exception.
- int rc = env->ThrowNew (exception_class, err_msg);
- env->DeleteLocalRef (exception_class);
-
- assert (rc == 0);
-}
-
-/**
* Called to construct a Java Socket object.
*/
JNIEXPORT void JNICALL Java_org_zmq_Socket_construct (JNIEnv *env,
jobject obj, jobject context, jint type)
{
- void *s = get_socket (env, obj);
- assert (! s);
+ void *s = get_socket (env, obj, 0);
+ if (s)
+ return;
void *c = fetch_context (env, context);
+ if (c == NULL) {
+ raise_exception (env, EINVAL);
+ return;
+ }
+
s = zmq_socket (c, type);
+ int err = errno;
put_socket(env, obj, s);
if (s == NULL) {
- raise_exception (env, errno);
+ raise_exception (env, err);
return;
}
}
@@ -131,14 +123,19 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_construct
(JNIEnv *env,
JNIEXPORT void JNICALL Java_org_zmq_Socket_finalize (JNIEnv *env,
jobject obj)
{
- void *s = get_socket (env, obj);
+ void *s = get_socket (env, obj, 0);
if (! s)
return;
int rc = zmq_close (s);
+ int err = errno;
s = NULL;
put_socket (env, obj, s);
- assert (rc == 0);
+
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
}
/**
@@ -156,13 +153,16 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_setsockopt__IJ
(JNIEnv *env,
case ZMQ_RECOVERY_IVL:
case ZMQ_MCAST_LOOP:
{
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
int64_t value = optval;
int rc = zmq_setsockopt (s, option, &value, sizeof (value));
- if (rc != 0)
- raise_exception (env, errno);
+ int err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
+
return;
}
default:
@@ -187,15 +187,22 @@ JNIEXPORT void JNICALL
Java_org_zmq_Socket_setsockopt__ILjava_lang_String_2 (
return;
}
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
const char *value = env->GetStringUTFChars (optval, NULL);
- assert (value);
+ if (! value) {
+ raise_exception (env, EINVAL);
+ return;
+ }
+
int rc = zmq_setsockopt (s, option, value, strlen (value));
+ int err = errno;
env->ReleaseStringUTFChars (optval, value);
- if (rc != 0)
- raise_exception (env, errno);
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
+
return;
}
default:
@@ -210,8 +217,7 @@ JNIEXPORT void JNICALL
Java_org_zmq_Socket_setsockopt__ILjava_lang_String_2 (
JNIEXPORT void JNICALL Java_org_zmq_Socket_bind (JNIEnv *env, jobject obj,
jstring addr)
{
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
if (addr == NULL) {
raise_exception (env, EINVAL);
@@ -225,10 +231,13 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_bind (JNIEnv
*env, jobject obj,
}
int rc = zmq_bind (s, c_addr);
+ int err = errno;
env->ReleaseStringUTFChars (addr, c_addr);
- if (rc == -1)
- raise_exception (env, errno);
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
}
/**
@@ -237,8 +246,7 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_bind (JNIEnv
*env, jobject obj,
JNIEXPORT void JNICALL Java_org_zmq_Socket_connect (JNIEnv *env,
jobject obj, jstring addr)
{
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
if (addr == NULL) {
raise_exception (env, EINVAL);
@@ -252,10 +260,13 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_connect
(JNIEnv *env,
}
int rc = zmq_connect (s, c_addr);
+ int err = errno;
env->ReleaseStringUTFChars (addr, c_addr);
- if (rc == -1)
- raise_exception (env, errno);
+ if (rc != 0) {
+ raise_exception (env, err);
+ return;
+ }
}
/**
@@ -264,36 +275,56 @@ JNIEXPORT void JNICALL Java_org_zmq_Socket_connect
(JNIEnv *env,
JNIEXPORT jboolean JNICALL Java_org_zmq_Socket_send (JNIEnv *env,
jobject obj, jbyteArray msg, jlong flags)
{
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
jsize size = env->GetArrayLength (msg);
- jbyte *data = env->GetByteArrayElements (msg, 0);
-
zmq_msg_t message;
int rc = zmq_msg_init_size (&message, size);
- assert (rc == 0);
- memcpy (zmq_msg_data (&message), data, size);
+ int err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return JNI_FALSE;
+ }
- env->ReleaseByteArrayElements (msg, data, 0);
+ jbyte *data = env->GetByteArrayElements (msg, 0);
+ if (! data) {
+ raise_exception (env, EINVAL);
+ return JNI_FALSE;
+ }
+ memcpy (zmq_msg_data (&message), data, size);
+ env->ReleaseByteArrayElements (msg, data, 0);
rc = zmq_send (s, &message, (int) flags);
+ err = errno;
- if (rc == -1 && errno == EAGAIN) {
+ if (rc != 0 && err == EAGAIN) {
rc = zmq_msg_close (&message);
- assert (rc == 0);
+ err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return JNI_FALSE;
+ }
return JNI_FALSE;
}
- if (rc == -1) {
- raise_exception (env, errno);
+ if (rc != 0) {
+ raise_exception (env, err);
rc = zmq_msg_close (&message);
- assert (rc == 0);
+ err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return JNI_FALSE;
+ }
return JNI_FALSE;
}
rc = zmq_msg_close (&message);
- assert (rc == 0);
+ err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return JNI_FALSE;
+ }
+
return JNI_TRUE;
}
@@ -303,29 +334,49 @@ JNIEXPORT jboolean JNICALL Java_org_zmq_Socket_send
(JNIEnv *env,
JNIEXPORT jbyteArray JNICALL Java_org_zmq_Socket_recv (JNIEnv *env,
jobject obj, jlong flags)
{
- void *s = get_socket (env, obj);
- assert (s);
+ void *s = get_socket (env, obj, 1);
zmq_msg_t message;
- zmq_msg_init (&message);
- int rc = zmq_recv (s, &message, (int) flags);
+ int rc = zmq_msg_init (&message);
+ int err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return NULL;
+ }
- if (rc == -1 && errno == EAGAIN) {
- zmq_msg_close (&message);
+ rc = zmq_recv (s, &message, (int) flags);
+ err = errno;
+ if (rc != 0 && err == EAGAIN) {
+ rc = zmq_msg_close (&message);
+ err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return NULL;
+ }
return NULL;
}
- if (rc == -1) {
- raise_exception (env, errno);
- zmq_msg_close (&message);
+ if (rc != 0) {
+ raise_exception (env, err);
+ rc = zmq_msg_close (&message);
+ err = errno;
+ if (rc != 0) {
+ raise_exception (env, err);
+ return NULL;
+ }
return NULL;
}
- jbyteArray data = env->NewByteArray (zmq_msg_size (&message));
- assert (data);
- env->SetByteArrayRegion (data, 0, zmq_msg_size (&message),
- (jbyte*) zmq_msg_data (&message));
+ // No errors are defined for these two functions. Should they?
+ int sz = zmq_msg_size (&message);
+ void* pd = zmq_msg_data (&message);
- return data;
+ jbyteArray data = env->NewByteArray (sz);
+ if (! data) {
+ raise_exception (env, EINVAL);
+ return NULL;
}
+ env->SetByteArrayRegion (data, 0, sz, (jbyte*) pd);
+ return data;
+}
_______________________________________________
zeromq-dev mailing list
[email protected]
http://lists.zeromq.org/mailman/listinfo/zeromq-dev
