[jbossts-issues] [JBoss JIRA] Commented: (JBTM-377) XAResourceRecord save/restore state insufficiently paranoid

[Jonathan Halliday (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBTM-377?page=comments#action_12419799 ] 
            
Jonathan Halliday commented on JBTM-377:
----------------------------------------

				try
				{
					if (_theXAResource instanceof Serializable)
						shouldSerialize = true;

					ByteArrayOutputStream s = new ByteArrayOutputStream();
					ObjectOutputStream o = new ObjectOutputStream(s);

					// don't allow the packing of a null resource

					if (_theXAResource == null)
						throw new NotSerializableException();

					o.writeObject(_theXAResource);
					o.close();

					os.packBoolean(true);

					os.packBytes(s.toByteArray());
				}
				catch (NotSerializableException ex)
				{
					if (!shouldSerialize)
					{
						// have to rely upon XAResource.recover!

						os.packBoolean(false);
					}
					else
					{
						if (jtaLogger.loggerI18N.isWarnEnabled())
						{
							jtaLogger.loggerI18N
									.warn("com.arjuna.ats.internal.jta.resources.arjunacore.savestate");
						}
					}
				}



Where _theXAResource does not implement Serializable, we call writeObject anyhow (arguably an error). Fortunately if (when?) it blows up, the exception handler reads shouldSerialize as false and packs the boolean correctly.

However, if _theXAResource does implement Serializable and yet writeObject still blows up (such as where the XAResource implementation is buggy and contains a non-serializable field) the written boolean is true. Hence we later attempt to deserialize the resource object in restore_state, reading whatever junk is in the objectstore at that location on disk and winding up with nonsense. This is particularly dangerous as part of the data is the size, used to allocate a byte array. Reading a junk value causes an arbitrarily large array to be requested, throws OutOfMemory and brings down the server.

Fix: the exception handler should write boolean false regardless of the value of the shouldSerialize flag, but print the warning message conditional on it.  Arguably we should also return false from the method, since we may be in a position where we lack sufficient information for successful recovery and therefore should not proceed with a tx commit.  Or we could just rely on resource driven recovery.

Regardless of the code path through other parts of save_state, we will always call super.save_state(os, t) before returning control, except if something goes horrifically wrong and results in a Throwable rather than an Exception. This point becomes important with regard to the implementation of restore_state.

restore_state is implemented such that it bails out (returns) in some cases without calling super.restore_state(os, t)  This matters little with regard to the internal state of the record, which is broken anyhow. However, it also critically leaves the underlying pointer into the log byte stream at the wrong location. Thus when BasicAction attempts to read the next resource record in sequence from the log, the read starts from an incorrect offset and reads junk, causing errors such as '13:26:18,877 WARN  [arjLoggerI18N] [com.arjuna.ats.internal.arjuna.gandiva.inventory.StaticInventory_1] -  cannot find null implementation.' because it can't map the junk value for record type into a sensible value. Fix: order the header first perhaps, or at least ensure the record is read fully before existing normally or otherwise from the restore_state method.



> XAResourceRecord save/restore state insufficiently paranoid
> -----------------------------------------------------------
>
>                 Key: JBTM-377
>                 URL: http://jira.jboss.com/jira/browse/JBTM-377
>             Project: JBoss Transaction Manager
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: JTA Implementation, Recovery
>    Affects Versions: 4.3.0.GA
>            Reporter: Jonathan Halliday
>         Assigned To: Mark Little
>             Fix For: 4.4.CR2
>
>
> The XAResourceRecord save state method writes meta-data into the objectstore before attempting to Serialize and write the actual bytes for an XAResource that it believes, on the basis of it implementing the marker interface, is Serializable. If the serialization fails at runtime, the objectstore record is left in an invalid state and can cause the recovery pass to blow up in horrible ways.
> We should obtain the byte[] representing the serialized resource before writing anything to the record so that this does not occur. If serialization fails, the record will therefore be useless but at least not otherwise broken.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira