views:

774

answers:

3

A hobby project of mine is a Java web application. It's a simple web page with a form. The user fills out the form, submits, and is presented with some results.

The data is coming over a JDBC Connection. When the user submits, I validate the input, build a "CREATE ALIAS" statement, a "SELECT" statement, and a "DROP ALIAS" statement. I execute them and do whatever I need to do with the ResultSet from the query.

Due to an issue with the ALIASes on the particular database/JDBC combination I'm using, it's required that each time the query is run, these ALIASes are created with a unique name. I'm using an int to ensure this which gets incremented each and every time we go to the database.

So, my data access class looks a bit like:

private final static Connection connection = // initialized however

private static int uniqueInvocationNumber = 0;

public static Whatever getData(ValidatedQuery validatedQuery) {
    String aliasName = "TEMPALIAS" + String.valueOf(uniqueInvocationNumber);
    // build statements, execute statements, deal with results
    uniqueInvocationNumber++;
}

This works. However, I've recently been made aware that I'm firmly stuck in Jon Skeet's phase 0 of threading knowledge ("Complete ignorance - ignore any possibility of problems.") - I've never written either threaded code or thread-aware code. I have absolutely no idea what can happen when many users are using the application at the same time.

So my question is, (assuming I haven't stumbled to thread-safety by blind luck / J2EE magic):

How can I make this safe?

I've included information here which I believe is relevant but let me know if it's not sufficient.

Thanks a million.

EDIT: This is a proper J2EE web application using the Wicket framework. I'm typically deploying it inside Jetty.

EDIT: A long story about the motivation for the ALIASes, for those interested:

The database in question is DB2 on AS400 (i5, System i, iSeries, whatever IBM are calling it these days) and I'm using jt400.

Although DB2 on AS400 is kind of like DB2 on any other platform, tables have a concept of a "member" because of legacy stuff. A member is kind of like a chunk of a table. The query I want to run is

SELECT thisField FROM thisTable(thisMember)

which treats thisMember as a table in its own right so just gives you thisField for all the rows in the member.

Now, queries such as this run fine in an interactive SQL session, but don't work over JDBC (I don't know why). The workaround I use is to do something like

CREATE ALIAS tempAlias FOR thisTable(thisMember)

then a

SELECT thisField FROM tempAlias

then a

DROP ALIAS tempAlias

which works but for one show-stopping issue: when you do this repeatedly with the ALIAS always called "tempAlias", and have a case where thisField has a different length from one query to the next, the result set comes back garbled for the second query (getString for the first row is fine, the next one has a certain number of spaces prepended, the next one the same number of spaces further prepended - this is from memory, but it's something like that).

Hence the workaround of ensuring each ALIAS has a distinct name which clears this up.

I've just realised (having spent the time to tap this explanation out) that I probably didn't spend enough time thinking about the issue in the first place before seizing on the workaround. Unfortunately I haven't yet fulfilled my dream of getting an AS400 for my bedroom ;) so I can't try anything new now.

A: 

If that static variable and the incrementing of the unique invocation number is visible to all requests, I'd say that it's shared state that needs to be synchronized.

duffymo
+8  A: 

Well, I'm going to ignore any SQL stuff for the moment and just concentrate on the uniqueInvocationNumber part. There are two problems here:

  • There's no guarantee that the thread will see the latest value at any particular point
  • The increment isn't atomic

The simplest way to fix this in Java is to use AtomicInteger:

private static final AtomicInteger uniqueInvocationNumber = new AtomicInteger();

public static Whatever getData(ValidatedQuery validatedQuery) {
    String aliasName = "TEMPALIAS" + uniqueInvocationNumber.getAndIncrement()
    // build statements, execute statements, deal with results
}

Note that this still assumes you're only running a single instance on a single server. For a home project that's probably a reasonable assumption :)

Another potential problem is sharing a single connection amongst different threads. Typically a better way of dealing with database connections is to use a connection pool, and "open/use/close" a connection where you need to (closing the connection in a finally block).

Jon Skeet
You really ought to put a `final` in there.
Tom Hawtin - tackline
@Tom: Done, thanks.
Jon Skeet
A: 

I know this doesn't answer your question but I would seriously consider re-implementing the feature so creating all those aliases isn't required. (Could you explain what kind of alias you're creating and why it's necessary?)

I understand this is just a hoby project, but consider putting on your 'to do list' to switch to using a connection pool. It's all part of the learning which I guess is part of your motivation for doing this project. Connection pools are the proper way to deal with multiple simultaneous users in a database backed web-app.

David Easley
Hi. Yes I will implement the connection pool idea, certainly before I let anyone else use it. If you're interested I'm about to add edit about the ALIASes above.