View Issue Details

IDProjectCategoryView StatusLast Update
0000427LDMud 3.5Networkingpublic2018-01-30 04:59
Reporterlynx Assigned Tozesstra  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Target Version3.5.0Fixed in Version3.5.0 
Summary0000427: wrong use of EMFILE for MAX_OUTCONN
Descriptionprobably comes from junky's original code. there is no distinguish
the temporary overflow of the outgoing connections queue from the
real case of an exhaustion of descriptors. in the first case it
would be ok to simply resubmit the net_connect request 2 seconds
later, in the latter case it is correct to either queue the operation
for better times, or bail out. so we need a different return code here
that helps us distinguish these two error states. too bad posix doesn't
have any to borrow us i'm afraid. can you make one up, please? :-)

the code in question is in comm.c and looks like this:

        if (!stored)
        {
            rc = EMFILE;
            break;
        }

as you see it manually creates an EMFILE error although it isn't
a real case of EMFILE. this was okay in the days when we only had
an irc robot and occasional gopher requests going out of the mud.
yeah, talking about 1992. but today we have jabber, psyc, http and
irc connections going in all directions..

an other possible solution would be to allocate the queue dynamically,
but do we really need to do so much work? having an extra error code
is the simpler & immediate solution.
Additional Informationalso, MAX_OUTCONN should really be configurable in the settings
file because for our needs 5 is way below mental sanity. just think
of a user logging into psyced with plenty of psyc and jabber friends
everywhere. one user alone may need a queue of dozens.

and thanks for making ldmud rock :)
TagsNo tags attached.

Activities

zesstra

2008-12-15 12:45

administrator   ~0000823

What about using ENOBUFS instead? It seems to me a similar cause (though in the kernel) and most probably a transient error as well.
BTW: Does anyone know, if there is any value range, which can never be used in errno?

lynx

2009-05-22 03:48

reporter   ~0001129

Since we no longer have MAX_OUTCONN limited to 5, this problem usually doesn't appear.. but still the code is there to handle EMFILE the "wrong" way.

I'd rather use a value unlikely to ever appear in errno, like -37337 ;)
But since I've never encountered a ENOBUFS it may as well be ENOBUFS.
Both solutions are fine with me.

zesstra

2009-05-22 05:41

administrator   ~0001135

The problem is, that the implementation of errno is not specified. (It may not even be a real variable.) The standard defines only 3 (symbolic) values, all other may be implementation-defined. It is very difficult to find one, which is guaranteed not to be used by someone else. Even if we assume, that all 'official' values are positive and use negatives ourself, this may go awfully wrong, if errno is implemented as 'unsigned'.
One possibility would be to abandon the idea of returning errno at all and define all error values ourselves, independent of errno.

lynx

2009-05-22 05:48

reporter   ~0001136

If our errno is signed and it only assigns some meaning to -37337, then it is quite unlikely that official values of errno will ever get there. But if you don't like that, take ENOBUFS please. Let's be pragmatic, we don't need an error code revolution here. In fact, if my OS introduces some new errnos, I prefer having them relayed into my LPC. For a while I even used to have an errno() efun, in order to be able to provide more precise error messages after file read/write ops etc. That was useful!

zesstra

2009-05-22 06:37

administrator   ~0001139

It is not so easy. Suppose, we choose
#define ENOSOCKET x
and return that in case we can't store the connection attempt.
Now the OS (future version) invents
#define ESOMESOCKETERROR x
Now you can't distinguish between these two error conditions any more.

On the second thought: returning errno, without supplying the OS version of errno.h and/or provide access to strerror() is completely nonsense. Why? Because the actual numbers are implementation defined. Example: EPROTONOSUPPORT on my system is 43. On MorgenGrauen it is 93.
If we want to follow this route, we have to create a /sys/errno.h on-the-fly upon driver startup. Which is not so easy, because the actual definition of the numbers may be in /usr/include/errno.h, /usr/include/sys/errno.h, /usr/include/asm-generic/errno-base.h, /usr/include/asm-generic/errno.h and probably other locations as well...

lynx

2009-05-22 07:39

reporter   ~0001145

I left it to the admin to create an errno.h for his system, if he likes to use the errno() efun.

Gnomi

2009-05-25 17:09

manager   ~0001155

I don't think that LPC should return an errno code, otherwise we will have to start allowing system includes. That's too low-level for LPC in my opinion, there should be a level of abstraction from the system functions. So we should either implement our own error codes (just like file_size() does), or throw textual representation (strerror(errno)) as a runtime error.

zesstra

2012-12-09 15:24

administrator   ~0002177

I agree. And I think, changing the meaning of the return values (and/or adding of additional RTEs) is better done in 3.5.x, so I will move this issue.

zesstra

2013-08-21 00:02

administrator   ~0002207

You probably won't believe it, but: I pushed some changes into master, which should solve this problem. net_connect() now uses our own symbolic return values defined in comm.h and should enable to distinguish between some transient and non-transient errors.

zesstra

2018-01-29 19:59

administrator   ~0002318

Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 1488 for details). Thank you for reporting!

zesstra

2018-01-29 22:57

administrator   ~0002369

Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 2817 for details). Thank you for reporting!

zesstra

2018-01-30 04:59

administrator   ~0002420

Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 3901 for details). Thank you for reporting!

Issue History

Date Modified Username Field Change
2005-12-16 12:26 lynx New Issue
2008-12-15 12:45 zesstra Note Added: 0000823
2009-05-22 03:48 lynx Note Added: 0001129
2009-05-22 05:41 zesstra Note Added: 0001135
2009-05-22 05:48 lynx Note Added: 0001136
2009-05-22 06:37 zesstra Note Added: 0001139
2009-05-22 07:39 lynx Note Added: 0001145
2009-05-25 17:09 Gnomi Note Added: 0001155
2011-02-23 23:02 zesstra Target Version => 3.3.721
2012-12-09 15:24 zesstra Note Added: 0002177
2012-12-09 15:24 zesstra Project LDMud 3.3 => LDMud 3.5
2012-12-09 15:25 zesstra Status new => confirmed
2012-12-09 15:25 zesstra Product Version 3.3.712 =>
2012-12-09 15:25 zesstra Target Version 3.3.721 => 3.5.0
2013-08-21 00:00 zesstra Assigned To => zesstra
2013-08-21 00:00 zesstra Status confirmed => assigned
2013-08-21 00:02 zesstra Note Added: 0002207
2013-08-21 00:02 zesstra Status assigned => resolved
2013-08-21 00:02 zesstra Fixed in Version => 3.5.0
2013-08-21 00:02 zesstra Resolution open => fixed
2018-01-29 19:59 zesstra Source_changeset_attached => ldmud.git master ba9f4da6
2018-01-29 19:59 zesstra Note Added: 0002318
2018-01-29 22:57 zesstra Source_changeset_attached => ldmud.git master ba9f4da6
2018-01-29 22:57 zesstra Note Added: 0002369
2018-01-30 04:59 zesstra Source_changeset_attached => ldmud.git master ba9f4da6
2018-01-30 04:59 zesstra Note Added: 0002420