1 Reply Latest reply on Dec 2, 2003 4:49 PM by jacyg

    Dynamic-QL not thread safe

    jacyg

      The implementation of dynamic-ql is not thread safe in 3.2.x (haven't tested in 4.x). Essentially, the problem is this: in the JDBCDynamicQLQuery execute() method, it compiles the dynamic jboss-ql, and calls super.setSQL();. That method call sets an instance variable in the super class. Then, after doing some more work, the execute() method eventually calls the super.execute() method. That method, in turn, eventually calls a method that uses the sql instance variable that was set via setSQL(). However, there is only one instance of the Query object for each finder per (not clear on this: container? jvm?). At any rate, if multiple threads are using the same dynamic-ql finder on the same ejb at the same time, the potential for a race condition exists, where one thread sets the sql variable, but by the time it runs the query, another thread has set it to something else. Clearly, this is very bad. I was able to replicate this behavior on 3.2.1 - 3.2.3. I have not found any other complaints about this behavior. Therefore, it seems quite likely to me that it has not been fixed, or even noticed. Attached to this message are rewrites of the queries that fix these problems. These rewrites are based on the 3.2.1 code-base (due to other issues we ran into with 3.2.2, we were still using 3.2.1; and 3.2.3 wasn't out for us to test before we started fixing this problem). If someone would like to take a look at these files and help merge them into the cvs-head, that would be much appreciated. Essentially, the fix consists of this: the JDBCAbstractQueryCommand is modified so that the only instance variables it has are ones which none of its subclasses will be modifying and which can be directly set in the constructor. A protected execute method is added that takes all the other parameters that used to be instance variables. A second abstract class, JDBCAbstractStaticSQLCommand, is added that extends JDBCAbstractQueryCommand. This class is the new super class for all the queries whose SQL is not dynamic (currently, everything but JDBCDynamicQLQuery). This class initializes all the instance variables in its constructor (so that the same efficiencies of only compiling the SQL statement once are still in play). It then uses those instance variables when executing the superclasses execute() method. Finally, JDBCDynamicQLQuery directly extends JDBCAbstractStaticSQLCommand and calls the execute method with its own dynamic (and locally scoped) variables. This seems to end all the mysterious errors we were getting that we traced back to using dynamic ql. Let me know if you have any questions, or concerns. I have been having trouble getting this post to go with the attachment, i'll try to attach the jared up source in a seperate post.