HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

HTTP server and multi-threading optimization

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
multiandoptimizationthreadinghttpserver

Problem

I wrote an HTTP server for the management of scores for users and levels.

It can return the highest score per level. It has a simple login with session-key.

What do you think could be improved in this code? What's wrong? And particularly, are there any multi-threading issues that I didn't take into account?

```
//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: demonstration of an http server
//
//***

package org.bombo.server;

import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.concurrent.Executors;

import com.sun.net.httpserver.HttpContext;
import com.sun.net.httpserver.HttpServer;

public class Main {

private static int port = 8009;

public static void main(String[] args) throws IOException {

// Create an http server
HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);

// Create a context
HttpContext context = server.createContext("/", new RequestsHandler());

// Add a filter
context.getFilters().add(new ParamsFilter());

// Set an Executor for the multi-threading
server.setExecutor(Executors.newCachedThreadPool());

// Start the server
server.start();

System.out.println("The server is started!");
}
}

//****
//
// Http Server
//
// Author: Bombo Gongo
//
// Class purpose: filter to set the request parameters
//
//***

package org.bombo.server;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.util.HashMap;
import java.util.Map;

import org.bombo.util.MiscUtils;

import com.sun.net.httpserver.*;

public class ParamsFilter extends Filter {

MiscUtils u

Solution


  • Please think twice before using your own http server. The better way is using Jetty or embed Tomcat - good way to get rid of defects and to save time for developing business logic.



-
If you're going to use multithreading please look into java.util.concurrent.atomic package in order to start using AtomicInteger etc.

-
Your multithreading is not actually multithreading as you've wrapped all logic into synchronized block. This will cause all threads to be executed one by one.

-
Map>> is not a good way of handling variables. This might be an object. Probably you can make it thread safe inside

-
volatile on Map is useless. It's like putting final on it. Anyway usage of these modifiers is not recommended as you can handle all types by concurrent package.

-
This point is only my opinion but I do not like using TreeMap. As far as I see you are trying to return sorted String. But you can get values from scoreResults (map.values()) and sort them with Collections.sort. Also you should look into StringUtils.join() method (apache.commons package) in order to receive String. This will give you more readable code.

-
Always close streams in try catch with finally block and always consume response body as you can receive memory leaks in your code:

os.write(response.toString().getBytes());

E.g. if the line above will cause an exception then os stream won't be closed. This will cause memory leak in your server. Please take a look in IOUtils.closeQuitely() method (apache.commons.io) as an example

Hope this will help you

Context

StackExchange Code Review Q#24740, answer score: 10

Revisions (0)

No revisions yet.