Re: [Xenomai-core] nucleus/pipe.c patch: check for lingering close at xnpipe_connect()

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 2501 bytes --]

Andreas Glatz wrote:
> Hi,
> 
> Currently, you can destroy RT_PIPE while a non-rt application is
> connected. If that happens the pipe won't be fully closed but
> some parts will be leftover for cleanup when closing the RT_PIPE
> from the non-rt application (lingering close).
> 
> For us it would be very convenient if we didn't have to close
> the non-rt side before restarting the rt-side (currently, the 
> rt-side complains bc the non-rt side didn't resolve the lingering
> close).
> 
> Here is a patch where the rt-side resolves the lingering close
> when connecting to the RT_PIPE. I tested it and it seems to work 
> fine. Do you have any concerns?
> 
> Andreas
> 

Whenever possible, please post patches inline (I had to manually copy it
to allow commenting).

> --- pipe.orig.c	2009-09-09 17:23:10.000000000 -0400
> +++ pipe.c	2009-09-09 17:32:48.000000000 -0400
> @@ -309,17 +309,35 @@
>  int xnpipe_connect(int minor, struct xnpipe_operations *ops, void *xstate)
>  {
>  	struct xnpipe_state *state;
> -	int need_sched = 0, ret;
> +	int need_sched = 0, test_lclose = 0, ret;
>  	spl_t s;
>  
> -	minor = xnpipe_minor_alloc(minor);
> -	if (minor < 0)
> +	ret = xnpipe_minor_alloc(minor);
> +	if (ret >= 0)
> +		minor = ret;
> +	else 
> +	if(ret == -EBUSY)
> +		test_lclose = 1;
> +	else 
>  		return minor;
>  
>  	state = &xnpipe_states[minor];
>  
>  	xnlock_get_irqsave(&nklock, s);
>  
> +	if(test_lclose) {
> +		if(testbits(state->status, XNPIPE_KERN_LCLOSE)) {
> +			clrbits(state->status, XNPIPE_KERN_LCLOSE);
> +			xnlock_put_irqrestore(&nklock, s);
> +			state->ops.release(state->xstate);

You cannot bluntly call release() here. You may not run in Linux context
while that handler demands this. Moreover, releasing the state is the
job of the NRT user that opened it and obviously still has a hand on it.

(And the locking is imbalanced here, but that is shadowed by the other
flaws.)

> +		} else {
> +			xnlock_put_irqrestore(&nklock, s);
> +			return -EBUSY;
> +		}
> +	}
> +			
> +	xnlock_get_irqsave(&nklock, s);
> +
>  	ret = xnpipe_set_ops(state, ops);
>  	if (ret) {
>  		xnlock_put_irqrestore(&nklock, s);

IIUC, you are looking for a way to keep a pipe open on the NRT side
while the RT user is disappearing and re-appearing? Not sure if this is
feasible at all with current xnpipe. But if it is, I think you rather
have to introduce some kind of reset on the affected xnpipe_state.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]