ENet Modifications
Entry posted by Drew_Benton ·
3,650 views
The recent thread about Netwerks (wrapper for ENet) got me thinking some more about the library. I decided to fire up the latest source distribution and get hacking away at it. In this blog post, I'll share some of the modifications I have done to the library to make it a bit more usable as a core networking library to build on top of.
Let me start off by saying that ENet is a pretty well designed low level library. I've always liked the design, which you can read about here. The documentation is rather sparse and the code has some areas where some more comments would help, but the source code is provided and it comes under a really friendly MIT-style license. There is some room for a few improvements with the library though.
1. Memory failure management - The library seems to have some mixed logic when it comes to memory allocation failures. There is code in certain portions that attempts to handle NULL pointers, but the main API function for allocation memory simple calls abort if a memory allocation fails. Naturally, in any program you don't want a library simply calling abort under the hood, leaving you and your users wondering what happened.
The first set of changes I've made is to make the library consistently handle NULL memory allocations. This means at any time, if a memory allocation fails, the library will fall through execution and error out of the top level API calls without bringing down your entire application. Of course, I still have some more testing to do to make sure I didn't miss anything, but so far so good.
For this set of modifications, I had to make a few API functions return error codes rather than nothing. This was done so at the first sign of any error, execution can be stopped by falling through rather than continuing operations and possibly corrupting vital data. I have added a Panic function that is called when a memory allocation failure is detected as well. I wanted to separate the responsibility of handling a critical error such as a memory allocation failure from the function that allocated memory.
At least now if the programmer wants their application to abort on a memory error, they can do so with being able to know that is what happened first rather than being left in the dark.
2. Packet Size Limitation - In my reply earlier, I mentioned something about how ENet supports any sized packets through the use of fragments, but this can be exploited by clients to crash your server by requesting too much memory too fast, which inevitably leads to abort being called and taking down the server. Such bugs have been found in commercial mmo servers and as a result, malicious users have been able to negativity impact business.
If you have the source code for ENet, all you have to do is find the line: totalLength = ENET_NET_TO_HOST_32 (command -> sendFragment.totalLength); Then, you can check against some predefined constant. If the packet is larger, I just return the result of enet_peer_disconnect (one of the functions I've modified to have a return value now, but under normal operations, it returns 0.)
A change such as this is pretty vital to ensure clients cannot crash your server with huge packet allocation requests.
3. Callback function tracking - For this modification, I've defined a few sets of macros to compile in the __FUNCTION__, __LINE__, and __FILE__ preprocessor commands to report the malloc and free callback activity. This was done so I can track where allocation/deallocation requests are made throughout the library. I won't keep this modification in since it's served its purpose. It did help in showing where memory allocations were occurring in high frequencies.
I'll take a look at changing the mechanics of memory allocation and deallocation in those areas to make the library more efficient and memory friendly (by using a boost pool for example). Other parts of the library only allocate memory once, so those areas are fine as-is. I like having debug messages outputted since trying to track through the Visual Studio debugger has a noticeable impact on performance when breakpoints trigger.
Well those are the 3 main things I worked on tonight with ENet. The first two things are vital to making the library more usable in a commercial environment. The third was just to further my understanding of the library itself. I'm currently adding in some random memory failure tests to ensure at any time if a memory allocation fails, I've added logic in all the areas to handle it properly. I need to review all the code manually as well, but I'll have to do that on a fresh set of eyes. Tonight I'll run some simple client sending variable sized messages to the server to check for any bugs.
I think I want to spend a little of time to get a better idea of how NetDog works through object replication by experimenting with an ENet base and build up my own system. I guess I could also check out RakNet's code, but I don't care for the code design.
[Edit 1]
After doing some testing, I had a few crashes whenever the server or client window was suspended and the resumed. I tracked down the bug to some logic I added to free packets on an error, but they were already being freed by the caller function. I checked all the code I added and double checked the logic of the calling function and removed my offending code where needed. Now, no more crashes! During that testing, I also decided to remove all the gotos in favor of returning a function call that wrapped the logic.
While doing that, I realized I need to work in a better system for error propagation through the use of a global panic flag. Adding that logic was not hard at all, since I just wrote a small function with a static variable that's called before the panic function is invoked. Looking through the code some more, I need to add more parameter checking to handle NULL pointers. I'll get started on that now.
[Edit 2]
I've been cleaning up the library and condensing it into one file recently. I had to reformat the spacing by hand, which was not fun, but at least it's in a format I'm happy with. There's bound to be parts I've missed, so I'll have to make many passes through it in the upcoming days on fresh sets of eyes to find those inconsistencies. Likewise, I have to do the same to check error handling mechanics for all functions.
I think I'll write a new public API for the library, and then wrap up calls to internal code with SEH exception handling to ensure if a fatal exception is generated inside the library, information about it will be able to be logged to help track down the problem.
4 Comments
Recommended Comments