Re: [Tarantool-patches] [PATCH v4 03/11] coio: do not allow parallel usage of coio

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Georgy Kirichenko <georgy@tarantool.org> [20/02/12 13:09]:
> Simultaneous usage of one coio from two or more fiber could lead
> to undefined behavior as coio routines are replacing awaiting fiber
> (a data member) and stopping watcher without any relevance if there
> any other users of the coio object. Such behavior could lead to
> an applier invalid stream issue #4040.
> The proposal is to disable an active coio reuse by returning a fake
> EINPROGRESS error.

I am not aware of any cases when coio is used by multiple fibers:
it's a violation of the coio contract.

If you suspect it is violated in some cases, please add the
scenario description to the commit comment. Then, to better 
identify the case, your patch should add an assert in debug mode.
For release mode, I suggest we set a more clear error so that it's
easy then to spot in the log file and act upon, not EINPROGRESS.
Something like ERR_OPEN_A_BUG.

> 
> Part of #980
> ---
>  src/lib/core/coio.cc | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/src/lib/core/coio.cc b/src/lib/core/coio.cc
> index e88d724d5..faa7e5bd5 100644
> --- a/src/lib/core/coio.cc
> +++ b/src/lib/core/coio.cc
> @@ -238,6 +238,17 @@ coio_connect_timeout(struct ev_io *coio, struct uri *uri, struct sockaddr *addr,
>  	tnt_raise(SocketError, sio_socketname(coio->fd), "connection failed");
>  }
>  
> +/* Do not allow to reuse coio by different fiber. */
> +static inline void
> +check_coio_in_use(struct ev_io *coio)
> +{
> +	if (ev_is_active(coio)) {
> +		errno = EINPROGRESS;
> +		tnt_raise(SocketError, sio_socketname(coio->fd),
> +			  "already in use");
> +	}
> +}
> +
>  /**
>   * Wait a client connection on a server socket until
>   * timedout.
> @@ -249,6 +260,7 @@ coio_accept(struct ev_io *coio, struct sockaddr *addr,
>  	ev_tstamp start, delay;
>  	coio_timeout_init(&start, &delay, timeout);
>  
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	while (true) {
> @@ -302,6 +314,7 @@ coio_read_ahead_timeout(struct ev_io *coio, void *buf, size_t sz,
>  
>  	ssize_t to_read = (ssize_t) sz;
>  
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	while (true) {
> @@ -399,6 +412,7 @@ coio_write_timeout(struct ev_io *coio, const void *buf, size_t sz,
>  	ev_tstamp start, delay;
>  	coio_timeout_init(&start, &delay, timeout);
>  
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	while (true) {
> @@ -461,6 +475,7 @@ coio_writev_timeout(struct ev_io *coio, struct iovec *iov, int iovcnt,
>  	struct iovec *end = iov + iovcnt;
>  	ev_tstamp start, delay;
>  	coio_timeout_init(&start, &delay, timeout);
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	/* Avoid a syscall in case of 0 iovcnt. */
> @@ -518,6 +533,7 @@ coio_sendto_timeout(struct ev_io *coio, const void *buf, size_t sz, int flags,
>  	ev_tstamp start, delay;
>  	coio_timeout_init(&start, &delay, timeout);
>  
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	while (true) {
> @@ -563,6 +579,7 @@ coio_recvfrom_timeout(struct ev_io *coio, void *buf, size_t sz, int flags,
>  	ev_tstamp start, delay;
>  	coio_timeout_init(&start, &delay, timeout);
>  
> +	check_coio_in_use(coio);
>  	CoioGuard coio_guard(coio);
>  
>  	while (true) {
> -- 
> 2.25.0

-- 
Konstantin Osipov, Moscow, Russia