You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@mina.apache.org by Steve Crane <sc...@newbay.com> on 2009/06/04 11:15:07 UTC
Bug in DefaultIoFuture.awaitUninterrruptibly()
Hi there,
DefaultIoFuture.awaitUninterrruptibly() calls await0(Log.MAX_VALUE,
false) which does:
long endTime = System.currentTimeMillis() + timeoutMillis;
i.e.,
long endTime = System.currentTimeMillis() + Long.MAX_VALUE;
This comes out <0 which leads to:
if (ready) {
return ready;
} else if (timeoutMillis <= 0) {
return ready;
}
Doesn't really matter what is returned here because the return value is
ignored by the caller:
try {
await0(Long.MAX_VALUE, false);
} catch ( InterruptedException ie) {
// Do nothing : this catch is just mandatory by contract
}
The right solution IMHO would be to use a timeout of -1 to wait forever
and 0 for a poll. However you might prefer:
long endTime = timeoutMillis;
if (endTime != Long.MAX_VALUE)
endTime += System.currentTimeMillis();
Cheers,
Steve
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Steve Crane <sc...@newbay.com>.
(BTW I think this is also related to the Shutdown Thread. It looks like
this pattern is used around the disposalFuture. See
AbstractIoService.dispose().)
On Thu, 2009-06-04 at 11:39 +0200, Emmanuel Lecharny wrote:
> Steve Crane wrote:
> > Hi there,
> >
> Good catch !!!
> > DefaultIoFuture.awaitUninterrruptibly() calls await0(Log.MAX_VALUE,
> > false) which does:
> >
> > long endTime = System.currentTimeMillis() + timeoutMillis;
> >
> > i.e.,
> > long endTime = System.currentTimeMillis() + Long.MAX_VALUE;
> >
> > This comes out <0 which leads to:
> >
> > if (ready) {
> > return ready;
> > } else if (timeoutMillis <= 0) {
> > return ready;
> > }
> >
> > Doesn't really matter what is returned here because the return value is
> > ignored by the caller:
> >
> > try {
> > await0(Long.MAX_VALUE, false);
> > } catch ( InterruptedException ie) {
> > // Do nothing : this catch is just mandatory by contract
> > }
> >
> > The right solution IMHO would be to use a timeout of -1 to wait forever
> > and 0 for a poll. However you might prefer:
> >
> > long endTime = timeoutMillis;
> > if (endTime != Long.MAX_VALUE)
> > endTime += System.currentTimeMillis();
> >
>
> I think that something like :
>
> private boolean await0(long timeoutMillis, boolean interruptable)
> throws InterruptedException {
> long endTime = System.currentTimeMillis() + timeoutMillis;
>
> if (endTime < 0) {
> endTime = Long.MAX_VALUE;
> }
> ...
>
I guess that would work.
> would be better.
>
> wdyt ?
> > Cheers,
> > Steve
> >
> >
> >
> >
>
>
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Emmanuel Lecharny <el...@apache.org>.
Steve Crane wrote:
> Hi there,
>
Good catch !!!
> DefaultIoFuture.awaitUninterrruptibly() calls await0(Log.MAX_VALUE,
> false) which does:
>
> long endTime = System.currentTimeMillis() + timeoutMillis;
>
> i.e.,
> long endTime = System.currentTimeMillis() + Long.MAX_VALUE;
>
> This comes out <0 which leads to:
>
> if (ready) {
> return ready;
> } else if (timeoutMillis <= 0) {
> return ready;
> }
>
> Doesn't really matter what is returned here because the return value is
> ignored by the caller:
>
> try {
> await0(Long.MAX_VALUE, false);
> } catch ( InterruptedException ie) {
> // Do nothing : this catch is just mandatory by contract
> }
>
> The right solution IMHO would be to use a timeout of -1 to wait forever
> and 0 for a poll. However you might prefer:
>
> long endTime = timeoutMillis;
> if (endTime != Long.MAX_VALUE)
> endTime += System.currentTimeMillis();
>
I think that something like :
private boolean await0(long timeoutMillis, boolean interruptable)
throws InterruptedException {
long endTime = System.currentTimeMillis() + timeoutMillis;
if (endTime < 0) {
endTime = Long.MAX_VALUE;
}
...
would be better.
wdyt ?
> Cheers,
> Steve
>
>
>
>
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Emmanuel Lecharny <el...@apache.org>.
Steve Crane wrote:
> Huh, I guess I wasn't logged in properly before. FYI it's DIRMINA-717.
>
Many thanks !
I will commit the patch asap.
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Steve Crane <sc...@newbay.com>.
Huh, I guess I wasn't logged in properly before. FYI it's DIRMINA-717.
Regards,
Steve
On Thu, 2009-06-04 at 12:15 +0200, Emmanuel Lecharny wrote:
> Steve Crane wrote:
> > I created an account at your JIRA but couldn't find a way to create a
> > ticket :-(
> >
> Could not be easier !
>
> Once you are connected (
> https://issues.apache.org/jira/secure/IssueNavigator.jspa ), on the top
> menu, select "Create New Issue", then on the two drop down list, select
> the project (mina) and the issue type (bug), then follow the workflow.
>
> Don't forget to select the version you have found the bug in.
>
> Thanks !
>
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Emmanuel Lecharny <el...@apache.org>.
Steve Crane wrote:
> I created an account at your JIRA but couldn't find a way to create a
> ticket :-(
>
Could not be easier !
Once you are connected (
https://issues.apache.org/jira/secure/IssueNavigator.jspa ), on the top
menu, select "Create New Issue", then on the two drop down list, select
the project (mina) and the issue type (bug), then follow the workflow.
Don't forget to select the version you have found the bug in.
Thanks !
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Steve Crane <sc...@newbay.com>.
I created an account at your JIRA but couldn't find a way to create a
ticket :-(
On Thu, 2009-06-04 at 11:40 +0200, Emmanuel Lecharny wrote:
> Forgot to ask you to create a JIRA for tracability sake, if you don't mind.
>
> Thanks !
>
> Steve Crane wrote:
> > Hi there,
> >
> > DefaultIoFuture.awaitUninterrruptibly() calls await0(Log.MAX_VALUE,
> > false) which does:
> >
> > long endTime = System.currentTimeMillis() + timeoutMillis;
> >
> > i.e.,
> > long endTime = System.currentTimeMillis() + Long.MAX_VALUE;
> >
> > This comes out <0 which leads to:
> >
> > if (ready) {
> > return ready;
> > } else if (timeoutMillis <= 0) {
> > return ready;
> > }
> >
> > Doesn't really matter what is returned here because the return value is
> > ignored by the caller:
> >
> > try {
> > await0(Long.MAX_VALUE, false);
> > } catch ( InterruptedException ie) {
> > // Do nothing : this catch is just mandatory by contract
> > }
> >
> > The right solution IMHO would be to use a timeout of -1 to wait forever
> > and 0 for a poll. However you might prefer:
> >
> > long endTime = timeoutMillis;
> > if (endTime != Long.MAX_VALUE)
> > endTime += System.currentTimeMillis();
> >
> > Cheers,
> > Steve
> >
> >
> >
> >
>
>
Re: Bug in DefaultIoFuture.awaitUninterrruptibly()
Posted by Emmanuel Lecharny <el...@apache.org>.
Forgot to ask you to create a JIRA for tracability sake, if you don't mind.
Thanks !
Steve Crane wrote:
> Hi there,
>
> DefaultIoFuture.awaitUninterrruptibly() calls await0(Log.MAX_VALUE,
> false) which does:
>
> long endTime = System.currentTimeMillis() + timeoutMillis;
>
> i.e.,
> long endTime = System.currentTimeMillis() + Long.MAX_VALUE;
>
> This comes out <0 which leads to:
>
> if (ready) {
> return ready;
> } else if (timeoutMillis <= 0) {
> return ready;
> }
>
> Doesn't really matter what is returned here because the return value is
> ignored by the caller:
>
> try {
> await0(Long.MAX_VALUE, false);
> } catch ( InterruptedException ie) {
> // Do nothing : this catch is just mandatory by contract
> }
>
> The right solution IMHO would be to use a timeout of -1 to wait forever
> and 0 for a poll. However you might prefer:
>
> long endTime = timeoutMillis;
> if (endTime != Long.MAX_VALUE)
> endTime += System.currentTimeMillis();
>
> Cheers,
> Steve
>
>
>
>
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org