View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000427 | LDMud 3.5 | Networking | public | 2005-12-16 11:26 | 2018-01-30 03:59 |
| Reporter | lynx | Assigned To | zesstra | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Target Version | 3.5.0 | Fixed in Version | 3.5.0 | ||
| Summary | 0000427: wrong use of EMFILE for MAX_OUTCONN | ||||
| Description | probably 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 Information | also, 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 :) | ||||
| Tags | No tags attached. | ||||
|
|
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? |
|
|
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. |
|
|
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. |
|
|
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! |
|
|
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... |
|
|
I left it to the admin to create an errno.h for his system, if he likes to use the errno() efun. |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 1488 for details). Thank you for reporting! |
|
|
Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 2817 for details). Thank you for reporting! |
|
|
Fix committed in revision ba9f4da6a3be62c9268f1a505436fc9e6d358236 to master branch (see changeset 3901 for details). Thank you for reporting! |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2005-12-16 11:26 | lynx | New Issue | |
| 2008-12-15 11:45 | zesstra | Note Added: 0000823 | |
| 2009-05-22 01:48 | lynx | Note Added: 0001129 | |
| 2009-05-22 03:41 | zesstra | Note Added: 0001135 | |
| 2009-05-22 03:48 | lynx | Note Added: 0001136 | |
| 2009-05-22 04:37 | zesstra | Note Added: 0001139 | |
| 2009-05-22 05:39 | lynx | Note Added: 0001145 | |
| 2009-05-25 15:09 | Gnomi | Note Added: 0001155 | |
| 2011-02-23 22:02 | zesstra | Target Version | => 3.3.721 |
| 2012-12-09 14:24 | zesstra | Note Added: 0002177 | |
| 2012-12-09 14:24 | zesstra | Project | LDMud 3.3 => LDMud 3.5 |
| 2012-12-09 14:25 | zesstra | Status | new => confirmed |
| 2012-12-09 14:25 | zesstra | Product Version | 3.3.712 => |
| 2012-12-09 14:25 | zesstra | Target Version | 3.3.721 => 3.5.0 |
| 2013-08-20 22:00 | zesstra | Assigned To | => zesstra |
| 2013-08-20 22:00 | zesstra | Status | confirmed => assigned |
| 2013-08-20 22:02 | zesstra | Note Added: 0002207 | |
| 2013-08-20 22:02 | zesstra | Status | assigned => resolved |
| 2013-08-20 22:02 | zesstra | Fixed in Version | => 3.5.0 |
| 2013-08-20 22:02 | zesstra | Resolution | open => fixed |
| 2018-01-29 18:59 | zesstra | Source_changeset_attached | => ldmud.git master ba9f4da6 |
| 2018-01-29 18:59 | zesstra | Note Added: 0002318 | |
| 2018-01-29 21:57 | zesstra | Source_changeset_attached | => ldmud.git master ba9f4da6 |
| 2018-01-29 21:57 | zesstra | Note Added: 0002369 | |
| 2018-01-30 03:59 | zesstra | Source_changeset_attached | => ldmud.git master ba9f4da6 |
| 2018-01-30 03:59 | zesstra | Note Added: 0002420 |
