From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Fri, 21 Dec 2018 14:04:56 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v2 3/4] session: store vtab in struct session
Message-ID: <20181221110456.7gavqguwgxx7ijoh@esperanza>
References: <cover.1544544183.git.v.shpilevoy@tarantool.org>
 <712b6f1f052126065eac6ca301221001c180906b.1544544183.git.v.shpilevoy@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <712b6f1f052126065eac6ca301221001c180906b.1544544183.git.v.shpilevoy@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Tue, Dec 11, 2018 at 07:10:22PM +0300, Vladislav Shpilevoy wrote:
> @@ -587,7 +587,8 @@ applier_f(va_list ap)
>  	 * Set correct session type for use in on_replace()
>  	 * triggers.
>  	 */
> -	if (session_create_on_demand(SESSION_TYPE_APPLIER) == NULL) {
> +	if (session_create_on_demand(SESSION_TYPE_APPLIER,
> +				     &generic_session_vtab) == NULL) {
>  		diag_log();
>  		return -1;
>  	}

> @@ -51,12 +52,12 @@ lbox_session_create(struct lua_State *L, enum session_type type)
>  {
>  	struct session *session = fiber_get_session(fiber());
>  	if (session == NULL) {
> -		session = session_create_on_demand(type);
> +		session = session_create_on_demand(type, &console_session_vtab);
>  		if (session == NULL)
>  			return luaT_error(L);
>  		session->meta.fd = lua_tointeger(L, 1);
>  	} else {
> -		session_set_type(session, type);
> +		session_set_type(session, type, &console_session_vtab);
>  	}

IMHO the new API is confusing, because it allows to do e.g.

  session_set_type(SESSION_TYPE_APPLIER, &console_session_vtab);

What about leaving the session vtab registry and using it in
session_set_type to set session->vtab consistent with a given type?
Then, to kill a session we would introduce a separate method, which
would reset session->vtab. And we could add an assertion ensuring that
session_set_type isn't called on a dead session.