You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2017/09/06 22:08:46 UTC
[1/3] qpid-proton git commit: PROTON-1574: Fix null pointers passed
to memmove
Repository: qpid-proton
Updated Branches:
refs/heads/master 790cada68 -> feafb6c80
PROTON-1574: Fix null pointers passed to memmove
Found by ASan sanitizer build
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/feafb6c8
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/feafb6c8
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/feafb6c8
Branch: refs/heads/master
Commit: feafb6c80b121f0a7ce87ea09c9b2f0f2d0fadf5
Parents: eac4e31
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Sep 6 15:48:43 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Sep 6 17:54:44 2017 -0400
----------------------------------------------------------------------
proton-c/src/core/buffer.c | 1 +
proton-c/src/core/framing.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/feafb6c8/proton-c/src/core/buffer.c
----------------------------------------------------------------------
diff --git a/proton-c/src/core/buffer.c b/proton-c/src/core/buffer.c
index c3015f4..96c6b37 100644
--- a/proton-c/src/core/buffer.c
+++ b/proton-c/src/core/buffer.c
@@ -163,6 +163,7 @@ int pn_buffer_ensure(pn_buffer_t *buf, size_t size)
int pn_buffer_append(pn_buffer_t *buf, const char *bytes, size_t size)
{
+ if (!size) return 0;
int err = pn_buffer_ensure(buf, size);
if (err) return err;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/feafb6c8/proton-c/src/core/framing.c
----------------------------------------------------------------------
diff --git a/proton-c/src/core/framing.c b/proton-c/src/core/framing.c
index 09bf4bb..18e3c38 100644
--- a/proton-c/src/core/framing.c
+++ b/proton-c/src/core/framing.c
@@ -94,7 +94,8 @@ size_t pn_write_frame(char *bytes, size_t available, pn_frame_t frame)
bytes[5] = frame.type;
pn_i_write16(&bytes[6], frame.channel);
- memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size);
+ if (frame.extended)
+ memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size);
memmove(bytes + 4*doff, frame.payload, frame.size);
return size;
} else {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[3/3] qpid-proton git commit: NO-JIRA: Restore valgrind defaults,
smaller values don't save much
Posted by ac...@apache.org.
NO-JIRA: Restore valgrind defaults, smaller values don't save much
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/5f4d852e
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/5f4d852e
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/5f4d852e
Branch: refs/heads/master
Commit: 5f4d852e19dbebba5d7cde1220c0ff57c3828507
Parents: 790cada
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Sep 6 12:23:09 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Sep 6 17:54:44 2017 -0400
----------------------------------------------------------------------
tools/py/proctest.py | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5f4d852e/tools/py/proctest.py
----------------------------------------------------------------------
diff --git a/tools/py/proctest.py b/tools/py/proctest.py
index 7a601e5..424cefd 100644
--- a/tools/py/proctest.py
+++ b/tools/py/proctest.py
@@ -59,11 +59,6 @@ class ProcError(Exception):
def __init__(self, proc, what="bad exit status"):
self.out = proc.out.strip()
returncode = getattr(proc, 'returncode') # Can be missing in some cases
- if returncode == proc.valgrind_exit:
- self.out = \
- "\n==NOTE== valgrind options set for speed, for more detail run command with" \
- "\n==NOTE== e.g. --leak-check=full --history-level=full --num-callers=40" \
- + self.out
msg = "%s (exit=%s) command:\n%s" % (what, returncode, " ".join(proc.args))
if self.out:
msg += "\nvvvvvvvvvvvvvvvv\n%s\n^^^^^^^^^^^^^^^^" % self.out
@@ -95,12 +90,11 @@ class Proc(Popen):
valgrind_exe = valgrind and os.getenv("VALGRIND")
if valgrind_exe:
# run valgrind for speed, not for detailed information
- vg = [valgrind_exe, "--quiet", "--num-callers=2",
- "--error-exitcode=%s" % self.valgrind_exit]
+ vg = [valgrind_exe, "--quiet", "--error-exitcode=%s" % self.valgrind_exit]
if helgrind:
- vg += ["--tool=helgrind", "--history-level=none"]
+ vg += ["--tool=helgrind"]
else:
- vg += ["--tool=memcheck", "--leak-check=full", "--leak-resolution=low"]
+ vg += ["--tool=memcheck", "--leak-check=full"]
self.args = vg + self.args
if os.getenv("PROCTEST_VERBOSE"):
sys.stderr.write("\n== running == "+" ".join(self.args)+"\n")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[2/3] qpid-proton git commit: PROTON-1574: Fix lock-order-inversion
warnings
Posted by ac...@apache.org.
PROTON-1574: Fix lock-order-inversion warnings
The lock introduced in
17d2a6f4 PROTON-1568: c++ enable race detection for self-tests
created lock-order problems.
Using the lock as a simple memory barrier solves the original race and the
lock-order problem.
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/eac4e310
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/eac4e310
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/eac4e310
Branch: refs/heads/master
Commit: eac4e3101a8e078e812e22b33b88b9b6219d8a5d
Parents: 5f4d852
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Sep 6 17:11:37 2017 -0400
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Sep 6 17:54:44 2017 -0400
----------------------------------------------------------------------
proton-c/src/proactor/epoll.c | 67 +++++++++++++++-----------------------
1 file changed, 27 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/eac4e310/proton-c/src/proactor/epoll.c
----------------------------------------------------------------------
diff --git a/proton-c/src/proactor/epoll.c b/proton-c/src/proactor/epoll.c
index 46effcc..6f0cc96 100644
--- a/proton-c/src/proactor/epoll.c
+++ b/proton-c/src/proactor/epoll.c
@@ -121,18 +121,25 @@ typedef enum {
// Data to use with epoll.
typedef struct epoll_extended_t {
- /* epoll_ctl()/epoll_wake() do not form a memory barrier, so cached memory
- writes to struct epoll_extended_t in the EPOLL_ADD thread might not be
- visible to epoll_wait() thread. Lock use of epoll_extended_t to be safe.
- */
- pmutex mutex;
struct psocket_t *psocket; // pconnection, listener, or NULL -> proactor
int fd;
epoll_type_t type; // io/timer/wakeup
uint32_t wanted; // events to poll for
bool polling;
+ pmutex barrier_mutex;
} epoll_extended_t;
+/* epoll_ctl()/epoll_wake() do not form a memory barrier, so cached memory
+ writes to struct epoll_extended_t in the EPOLL_ADD thread might not be
+ visible to epoll_wait() thread. This function creates a memory barrier,
+ called before epoll_ctl() and after epoll_wait()
+*/
+static void memory_barrier(epoll_extended_t *ee) {
+ // Mutex lock/unlock has the side-effect of being a memory barrier.
+ lock(&ee->barrier_mutex);
+ unlock(&ee->barrier_mutex);
+}
+
/*
* This timerfd logic assumes EPOLLONESHOT and there never being two
* active timeout callbacks. There can be multiple (or zero)
@@ -282,33 +289,26 @@ PN_STRUCT_CLASSDEF(pn_listener, CID_pn_listener)
static bool start_polling(epoll_extended_t *ee, int epollfd) {
if (ee->polling)
return false;
- pmutex_init(&ee->mutex);
- lock(&ee->mutex);
ee->polling = true;
struct epoll_event ev;
ev.data.ptr = ee;
ev.events = ee->wanted | EPOLLONESHOT;
- int fd = ee->fd;
- unlock(&ee->mutex);
- return (epoll_ctl(epollfd, EPOLL_CTL_ADD, fd, &ev) == 0);
+ memory_barrier(ee);
+ return (epoll_ctl(epollfd, EPOLL_CTL_ADD, ee->fd, &ev) == 0);
}
static void stop_polling(epoll_extended_t *ee, int epollfd) {
// TODO: check for error, return bool or just log?
- lock(&ee->mutex);
if (ee->fd == -1 || !ee->polling || epollfd == -1)
return;
struct epoll_event ev;
ev.data.ptr = ee;
ev.events = 0;
- unlock(&ee->mutex);
+ memory_barrier(ee);
if (epoll_ctl(epollfd, EPOLL_CTL_DEL, ee->fd, &ev) == -1)
EPOLL_FATAL("EPOLL_CTL_DEL", errno);
- lock(&ee->mutex);
ee->fd = -1;
ee->polling = false;
- unlock(&ee->mutex);
- pmutex_finalize(&ee->mutex);
}
/*
@@ -642,11 +642,9 @@ static void psocket_gai_error(psocket_t *ps, int gai_err, const char* what) {
static void rearm(pn_proactor_t *p, epoll_extended_t *ee) {
struct epoll_event ev;
ev.data.ptr = ee;
- lock(&ee->mutex);
ev.events = ee->wanted | EPOLLONESHOT;
- int fd = ee->fd;
- unlock(&ee->mutex);
- if (epoll_ctl(p->epollfd, EPOLL_CTL_MOD, fd, &ev) == -1)
+ memory_barrier(ee);
+ if (epoll_ctl(p->epollfd, EPOLL_CTL_MOD, ee->fd, &ev) == -1)
EPOLL_FATAL("arming polled file descriptor", errno);
}
@@ -1185,11 +1183,9 @@ static void pconnection_start(pconnection_t *pc) {
start_polling(&pc->timer.epoll_io, efd); // TODO: check for error
epoll_extended_t *ee = &pc->psocket.epoll_io;
- lock(&ee->mutex);
ee->fd = pc->psocket.sockfd;
ee->wanted = EPOLLIN | EPOLLOUT;
ee->polling = false;
- unlock(&ee->mutex);
start_polling(ee, efd); // TODO: check for error
}
@@ -1256,6 +1252,8 @@ void pn_proactor_connect(pn_proactor_t *p, pn_connection_t *c, const char *addr)
lock(&pc->context.mutex);
proactor_add(&pc->context);
+ pn_connection_open(pc->driver.connection); /* Auto-open */
+
bool notify = false;
bool notify_proactor = false;
@@ -1621,14 +1619,11 @@ void pn_listener_accept(pn_listener_t *l, pn_connection_t *c) {
/* Set up an epoll_extended_t to be used for wakeup or interrupts */
static void epoll_wake_init(epoll_extended_t *ee, int eventfd, int epollfd) {
- pmutex_init(&ee->mutex);
- lock(&ee->mutex);
ee->psocket = NULL;
ee->fd = eventfd;
ee->type = WAKE;
ee->wanted = EPOLLIN;
ee->polling = false;
- unlock(&ee->mutex);
start_polling(ee, epollfd); // TODO: check for error
}
@@ -1640,7 +1635,6 @@ pn_proactor_t *pn_proactor() {
pmutex_init(&p->eventfd_mutex);
pmutex_init(&p->bind_mutex);
ptimer_init(&p->timer, 0);
- pmutex_init(&p->overflow_mutex);
if ((p->epollfd = epoll_create(1)) >= 0) {
if ((p->eventfd = eventfd(0, EFD_NONBLOCK)) >= 0) {
@@ -1822,11 +1816,7 @@ static bool proactor_remove(pcontext_t *ctx) {
}
static pn_event_batch_t *process_inbound_wake(pn_proactor_t *p, epoll_extended_t *ee) {
- lock(&ee->mutex);
- int fd = ee->fd;
- unlock(&ee->mutex);
-
- if (fd == p->interruptfd) { /* Interrupts have their own dedicated eventfd */
+ if (ee->fd == p->interruptfd) { /* Interrupts have their own dedicated eventfd */
(void)read_uint64(p->interruptfd);
rearm(p, &p->epoll_interrupt);
return proactor_process(p, PN_PROACTOR_INTERRUPT);
@@ -1871,28 +1861,25 @@ static pn_event_batch_t *proactor_do_epoll(struct pn_proactor_t* p, bool can_blo
}
assert(n == 1);
epoll_extended_t *ee = (epoll_extended_t *) ev.data.ptr;
+ memory_barrier(ee);
- lock(&ee->mutex);
- epoll_type_t type = ee->type;
- psocket_t* psocket = ee->psocket;
- unlock(&ee->mutex);
- if (type == WAKE) {
+ if (ee->type == WAKE) {
batch = process_inbound_wake(p, ee);
- } else if (type == PROACTOR_TIMER) {
+ } else if (ee->type == PROACTOR_TIMER) {
batch = proactor_process(p, PN_PROACTOR_TIMEOUT);
} else {
- pconnection_t *pc = psocket_pconnection(psocket);
+ pconnection_t *pc = psocket_pconnection(ee->psocket);
if (pc) {
- if (type == PCONNECTION_IO) {
+ if (ee->type == PCONNECTION_IO) {
batch = pconnection_process(pc, ev.events, false, false);
} else {
- assert(type == PCONNECTION_TIMER);
+ assert(ee->type == PCONNECTION_TIMER);
batch = pconnection_process(pc, 0, true, false);
}
}
else {
// TODO: can any of the listener processing be parallelized like IOCP?
- batch = listener_process(psocket, ev.events);
+ batch = listener_process(ee->psocket, ev.events);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org