Hi,

> I need to know how we go about submitting patches and also to ask questions in case I am missing something
>
> A.) Destroy()
>
> Why are we throwing an exception if we have problems destroying the connection at 1.). By the time we return reach 2.) the physical connection is completely destroyed due to the finally in iscDetach() or due to a network error. This means the connection is no longer usable. It seems that in the JBoss connection manager this stops the connection from being properly destroyed and it gets recycled with devastating results. If we are interested in recognising the problem, can't we just log it instead. We even destroy out link to the underlying connection at 3. so we have no hope of reusing the connection.

Good question. Our code needs fixing, however I'm not sure that ignoring
the exception here is also correct. Reason is that destroy() method is
supposed to throw an ResourceException and according to the JCA 1.0
specification (pages 153-154) we should throw either
ResourceAdapterInternalException or CommException in case of
communication errors.

So the question is how do the other servers behave... Might be that
WebLogic, WebSphere and Geronimo would work differently.

If that were a single place, I would agree to suppress the error,
however your third item seems to be JBoss specific as well, therefore
possible solution might be creating a separate package with JBoss-tuned
classes and create a JBoss-tuned RAR file. We would have then one
generic packaging and one tuned for JBoss.

> Another issue the line 1.) which allows access to the underlying ApiHandler breaks encapsulation of the gdsHelper. This would normally be a style issue, but we have made network errors fatal. Typically helper functions in GDSHelper have the following format:
> ...
> Complete code for changes to GdsHelper including proxy attached.

I agree that encapsulation should be fixed. However, the GDSProxy
approach won't work, since there's only one GDS instance per
ManagedConnectionFactory. The only correct fix would be to move that
method into GDSHelper class, instances of which are bound to a
particular ManagedConnection objects.

> 3.) Another problem I have being experiencing with the JBoss transaction manager and Jaybird is that when a attempt to start a transaction fails, it will always fail because the container will keep on calling start() before using the connection. Becasue we have not notifed the container the connection is broken it will keep on cycling it. It would be better to have connection discarded, because if we won't allow a connection to start something has gone badly wrong between Jaybird and the container. The chances are we will never get past this stage no matter how many times we try. We may as well notify the container the the connection is no longer usable. We should do this even if the GDSException is not one of the 3 fatal errors. The possible changes for start are given below:

I think we do have to distinguish between fatal and not fatal errors, we
have to call ConnectionEventListener.connectionErrorOccured for latter,
but I see no reason to do this for the former ones.

However, we have an issue there - the
FBManagedConnection.findIscTrHandle(...) method also uses
gdsHelper.getInternalAPIHandler().XXX calls, and in our case no checks
for fatal errors happen. This has to be fixed.

Though, I'm not sure that this would do the trick in your case. So, very
likely, we would need JBoss-specific version of our JCA code.

Roman

P.S. ...some time ago I thought that the JCA code was pretty clean. How
wrong I was.