Hi Cliff,
On 4. 09. 14 20:03, Cliff Jansen wrote:
Hi,
I am not sure what you are trying to do with xdispatch.
Exactly the same idea as contrib/proton-hawtdispatch, but in C.
If you are
using and external loop and managing your own IO, you can bypass the
low level Proton IO primitives and access the proton engine directly.
This is how Qpid uses Proton.
I am using the engine directly.
I am also using io.[ch] as a *very* convenient multi platform socket
abstraction.
If you were using the driver.c mechanism, please take a look at
PROTON-658. This and the main messenger loop show how the
selectors/selectables can be used within Proton. pn_selector_select()
is pretty central to the completion port model on windows.
I'm not using the proton driver, I have my own driver and thread model.
All serialization of one engine instance is one serial libdispatch task
queue.
I'm also not using proton selectors but two libdispatch sources (read
and write)
that are bound to the same task queue.
All bits of my application that need to be talk to the engine then just
queue
tasks to the same libdispatch queue, so we have practically no locks in
the app.
You may find that if you use Windows accept and connect instead of
pn_accept and pn_connect you can keep your descriptors completely
outside the completion port model and use things as you did. If not,
perhaps we can find another way to isolate your sockets from the
completion port implementation.
I like io's eror thing, and the wouldblock thing, I'd hate to duplicate
all that
across the platforms.
Today I had a read through the iocp change.
I see it kept support for 'user supplied' sockets, where it behaves
synchronously, as before.
I can make iocp support be controlled by an opt-out parameter.
Then for example, pn_listen does not enroll the socket to iocp and
pn_accept
does not find it in the map and treat it as user supplied.
Patch attached, with it my code works again.
Would such a change be acceptable?
The patch is a bit rough
- not sure I really like the name pn_io_no_iocp()
- missing non-windows stub for pn_io_no_iocp()
- it most likely handles the selector access wrong (see XXX comment)
The Windows completion port implementation is designed to provide a
highly scalable throughput in a constrained (mostly) single thread
model, but remain lightweight for small clients. Central to that is
the integration with selectors and selectables. But it can't be
everything to everybody.
I did not want to say this is a bad thing! :) I just wanted to let you
guys know
there is a user of the library with slightly different use-case and to
figure out if
such use can be considered to be within scope of the things that proton
is trying to accomplish.
Cheers,
Bozzo
From 357bf8fa698e9a49170b5e53b9abb2a74007dbb2 Mon Sep 17 00:00:00 2001
From: Bozo Dragojevic <[email protected]>
Date: Fri, 5 Sep 2014 11:40:57 +0200
Subject: [PATCH 1/1] Make iocp optional on windows
---
proton-c/include/proton/io.h | 1 +
proton-c/src/windows/io.c | 47 ++++++++++++++++++++++++++++++++++----------
proton-c/src/windows/iocp.c | 8 ++++++++
proton-c/src/windows/iocp.h | 1 +
4 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/proton-c/include/proton/io.h b/proton-c/include/proton/io.h
index 2d56736..3aac524 100644
--- a/proton-c/include/proton/io.h
+++ b/proton-c/include/proton/io.h
@@ -47,6 +47,7 @@ typedef struct pn_io_t pn_io_t;
typedef struct pn_selector_t pn_selector_t;
PN_EXTERN pn_io_t *pn_io(void);
+PN_EXTERN bool pn_io_no_iocp(pn_io_t *io);
PN_EXTERN void pn_io_free(pn_io_t *io);
PN_EXTERN pn_error_t *pn_io_error(pn_io_t *io);
PN_EXTERN pn_socket_t pn_connect(pn_io_t *io, const char *host, const char
*port);
diff --git a/proton-c/src/windows/io.c b/proton-c/src/windows/io.c
index b5660be..f9c85b4 100644
--- a/proton-c/src/windows/io.c
+++ b/proton-c/src/windows/io.c
@@ -92,7 +92,8 @@ void pn_io_finalize(void *obj)
{
pn_io_t *io = (pn_io_t *) obj;
pn_error_free(io->error);
- pn_free(io->iocp);
+ if (io->iocp != NULL)
+ pn_free(io->iocp);
WSACleanup();
}
@@ -112,6 +113,20 @@ void pn_io_free(pn_io_t *io)
pn_free(io);
}
+bool pn_io_no_iocp(pn_io_t *io)
+{
+ if (io->iocp != NULL) {
+ if (pni_iocpdesc_map_empty(io->iocp)) {
+ pn_free(io->iocp);
+ io->iocp = NULL;
+ } else {
+ pn_error_format(io->error, PN_ERR, "Cannot turn off iocp, already in
use");
+ return false;
+ }
+ }
+ return true;
+}
+
pn_error_t *pn_io_error(pn_io_t *io)
{
assert(io);
@@ -210,14 +225,17 @@ pn_socket_t pn_listen(pn_io_t *io, const char *host,
const char *port)
return INVALID_SOCKET;
}
- iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
- if (!iocpd) {
- pn_i_error_from_errno(io->error, "register");
- closesocket(sock);
- return INVALID_SOCKET;
+ if (io->iocp != NULL) {
+ iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
+ if (!iocpd) {
+ pn_i_error_from_errno(io->error, "register");
+ closesocket(sock);
+ return INVALID_SOCKET;
+ }
+
+ pni_iocpdesc_start(iocpd);
}
- pni_iocpdesc_start(iocpd);
return sock;
}
@@ -242,7 +260,12 @@ pn_socket_t pn_connect(pn_io_t *io, const char *hostarg,
const char *port)
ensure_unique(io, sock);
pn_configure_sock(io, sock);
- return pni_iocp_begin_connect(io->iocp, sock, addr, io->error);
+ if (io->iocp != NULL) {
+ sock = pni_iocp_begin_connect(io->iocp, sock, addr, io->error);
+ } else {
+ freeaddrinfo(addr);
+ }
+ return sock;
}
pn_socket_t pn_accept(pn_io_t *io, pn_socket_t listen_sock, char *name, size_t
size)
@@ -341,6 +364,8 @@ bool pn_wouldblock(pn_io_t *io)
pn_selector_t *pn_io_selector(pn_io_t *io)
{
+ if (io->iocp == NULL)
+ return NULL; // XXX: can we do better here? not familiar with selectors....
if (io->iocp->selector == NULL)
io->iocp->selector = pni_selector_create(io->iocp);
return io->iocp->selector;
@@ -351,8 +376,10 @@ static void configure_pipe_socket(pn_io_t *io, pn_socket_t
sock)
u_long v = 1;
ioctlsocket (sock, FIONBIO, &v);
ensure_unique(io, sock);
- iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
- pni_iocpdesc_start(iocpd);
+ if (io->iocp != NULL) {
+ iocpdesc_t *iocpd = pni_iocpdesc_create(io->iocp, sock, false);
+ pni_iocpdesc_start(iocpd);
+ }
}
diff --git a/proton-c/src/windows/iocp.c b/proton-c/src/windows/iocp.c
index 614b130..71fbc10 100644
--- a/proton-c/src/windows/iocp.c
+++ b/proton-c/src/windows/iocp.c
@@ -768,10 +768,18 @@ iocpdesc_t *pni_iocpdesc_create(iocp_t *iocp, pn_socket_t
s, bool external) {
// === Fast lookup of a socket's iocpdesc_t
iocpdesc_t *pni_iocpdesc_map_get(iocp_t *iocp, pn_socket_t s) {
+ if (iocp == NULL)
+ return NULL;
iocpdesc_t *iocpd = (iocpdesc_t *) pn_hash_get(iocp->iocpdesc_map, s);
return iocpd;
}
+bool pni_iocpdesc_map_empty(iocp_t *iocp) {
+ if (iocp == NULL)
+ return true;
+ return !pn_hash_size(iocp->iocpdesc_map);
+}
+
void pni_iocpdesc_map_push(iocpdesc_t *iocpd) {
pn_hash_put(iocpd->iocp->iocpdesc_map, iocpd->socket, iocpd);
pn_decref(iocpd);
diff --git a/proton-c/src/windows/iocp.h b/proton-c/src/windows/iocp.h
index bc64dd0..5e6b09c 100644
--- a/proton-c/src/windows/iocp.h
+++ b/proton-c/src/windows/iocp.h
@@ -104,6 +104,7 @@ struct write_result_t {
iocpdesc_t *pni_iocpdesc_create(iocp_t *, pn_socket_t s, bool external);
iocpdesc_t *pni_iocpdesc_map_get(iocp_t *, pn_socket_t s);
+bool pni_iocpdesc_map_empty(iocp_t *iocp);
void pni_iocpdesc_map_del(iocp_t *, pn_socket_t s);
void pni_iocpdesc_map_push(iocpdesc_t *iocpd);
void pni_iocpdesc_start(iocpdesc_t *iocpd);
--
1.9.2.msysgit.0
