Using DatagramChannel.write(...) without bind the channel fails.

[normanmaurer]

It's completely legal to use DatagramChannel.write(new DatagramPacket) even if its not bound. At the moment this fails with NotYetConnectedException in doFlush(..).

This is because NioDatagramChannel.isActive() returns false if its not bound. The same is true for OioDatagramChannel. I think isActive should return true as long as it was not closed yet.

[trustin]

Do we now trigger channelActive() immediately on registration?

[normanmaurer]

Its triggered when registration was done and Channel.isActive() returns true

Am 11.09.2013 um 21:43 schrieb Trustin Lee notifications@github.com:

Do we now trigger channelActive() immediately on registration?

—
Reply to this email directly or view it on GitHub.

[CodeMason]

This changed completely breaks "normal" Datagram Channels, I'm searching for either a work around or a better solution.

Since isActive() is immediately triggered now, anything in the pipeline handler that is dependent on the remote address will fail, as it is no longer assigned when fireChannelActive() is triggered.

[normanmaurer]

@CodeMason hmm.. I think in fact a DatagramChannel is active directly what it is not is "connected". Maybe we should fire an userEvent once it is connected in this case ?
WDYT ?

Anyway I think I need to revert this change and implement it somehow different in 4.0 for backward compatibility.

[CodeMason]

I think you mean that a DatagramChannel is "active" as soon as it is open? That makes sense if there is no remoteAddress option specified. If there is a remoteAddress specified, then "active" means it's open and bound.

WDYT?

I agree that this change should be reverted until a solution is found that keeps backward compatability.

[CodeMason]

About firing a userEvent once it's connected, what do you mean by that?

[normanmaurer]

call fireUserEvent(DatagramChannelConnectedEvent). This way a user can intercept it via override userEventTriggered(..) in ChannelInboundHandler

[CodeMason]

having to intercept that way is not intuitive at all.

What do you think about changing what bootstrap.connect() does? "isActive = open + bound" only matters for DatagramChannel when trying to connect/bind.

So if connect is called before write, then isActive changes behavior to be open + bound, otherwise isActive = open?

[normanmaurer]

@CodeMason not sure I understand what you propose... Can you give more details please .

[CodeMason]

What I understand the problem is:

Using Datagram.channel.write() without binding first will fail, this is
because isActive() = ch.isOpen() && ch.socket().isBound(). This fails
because Bootstrap.connect or Bootstrap.bind are not used and
ch.socket().isBound() will be false, and therefore, isActive (in it's
original version) fails.

What I propose:

An additional field will be added to the NioDatagramChannel class "boolean
ignoreBoundState = true". Bootstrap.connect() and bootstrap.bind() will
change the behaviour of channel.isActive by setting ignoreBoundState =
false.

Instead of:
isActive = ch.isOpen() && ch.socket().isBound()

it will be:
isActive = ch.isOpen() && (ignoreBoundState || ch.socket().isBound())

This way, if Datagram.channel.write() is called without connect/bind being
called first, then it will succeed -- since ignoreBoundState is what we
want. If bootstrap.connect/bind is called first, then the behaviour of
isActive() will make sure the channel is bound before triggering
fireChannelActive (and will now be backwards compatible).

I think this will work, because fireChannelActive where the problem is
(when caring about bound channels), and the proposed change will return
isActive (and consequently fireChannelActive) to the old behaviour if
bootstrap.connect/bind are used, yet still permit Datagram.channel.write()
without having to call bootstrap.connect/bind first.

WDYT?

On Mon, Sep 23, 2013 at 3:42 AM, Norman Maurer notifications@github.comwrote:

@CodeMason https://github.com/CodeMason not sure I understand what you
propose... Can you give more details please .

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1830#issuecomment-24903235
.

[normanmaurer]

@CodeMason I used a ChannelOption to change behavior. I think this is more clean, I still think about what to do best for master. But please review...

The default behavior should be the same as previous 4.0.x releases now

[CodeMason]

All my unit tests pass, thank you.

You're right, that is a lot more clean, and I like having it be an option to change the behavior.

The only other thing I can think of (which may or may not be a good idea), is to have a different Channel type, meaning that in addition to NioDatagramChannel.class/OioDatagramChannel.class, there would be NioDatagramUnboundChannel.class/NioDatagramUnboundChannel.class.

The Unbound variants would have the channel active as soon as it is bound to the EventLoop.