Hi,
Sorry for this late reply.
This is more than just coding style, as this class contains
reentrant code without synchronization.
I decided to write the code that way to avoid a race between
the setDone() method called on tx commit/rollback and most
of the other methods here, that would result in a spurious
NPE if for example the transaction is rolled back at the
same time as getStatus() if called.
The problem is that JTA imposes almost no restrictions on
the allowed calling sequences for a Transaction.
Fortunately, txCapsule is already declared volatile, or
this change could have opened some nasty races, like
committing the wrong transaction (txCapsule instances are
reused).
True, NPEs may be thrown from XA resources and tx
synchronizations, but these are caught and dealt with
in TxCapsule. This still leaves the (small) possibility
that a bug in TxCapsule could throw an NPE that is
mistakenly caught in TransactionImpl.
So, if the sole reason for this change is coding style,
I would like to change this back and add some comments
about why I do "try {} catch(NPE)".
This way we don't risk spurious NPEs, if methods like
getStatus() race with commit() or rollback().
(But I have to wait a few days until I am up-to-date on
the current CVS head until I start committing.)
Best Regards,
Ole Husgaard.
Scott M Stark wrote:
>
> I would say that is a better style. Fixed.
>
> xxxxxxxxxxxxxxxxxxxxxxxx
> Scott Stark
> Chief Technology Officer
> JBoss Group, LLC
> xxxxxxxxxxxxxxxxxxxxxxxx
> ----- Original Message -----
> From: "Oleg Nitz" <[EMAIL PROTECTED]>
> To: <[EMAIL PROTECTED]>
> Sent: Monday, May 27, 2002 1:35 PM
> Subject: [JBoss-dev] NPE use in TransactionImpl.java
>
> > Hi!
> >
> > Here is a result of peer-review of TransactionImpl.java :)
> > Look at this:
> >
> > public int getStatus()
> > throws SystemException
> > {
> > try {
> > return txCapsule.getStatus();
> > } catch (NullPointerException ex) {
> > return Status.STATUS_NO_TRANSACTION;
> > }
> > }
> >
> > NPE here is thrown quite often, at least I noticed this with JBoss 2.4.6
> > Wouldn't it be better to code this in the following way?
> >
> > public int getStatus()
> > throws SystemException
> > {
> > if (txCapsule != null) {
> > return txCapsule.getStatus();
> > } else {
> > return Status.STATUS_NO_TRANSACTION;
> > }
> > }
> >
> > Regards,
> > Oleg
> >
> > _______________________________________________________________
> >
> > Don't miss the 2002 Sprint PCS Application Developer's Conference
> > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
> >
> > _______________________________________________
> > Jboss-development mailing list
> > [EMAIL PROTECTED]
> > https://lists.sourceforge.net/lists/listinfo/jboss-development
> >
>
> _______________________________________________________________
>
> Don't miss the 2002 Sprint PCS Application Developer's Conference
> August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
>
> _______________________________________________
> Jboss-development mailing list
> [EMAIL PROTECTED]
> https://lists.sourceforge.net/lists/listinfo/jboss-development
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Jboss-development mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/jboss-development
