[QFJ-845] Change packaging strategy of quickfixj-messages-all to prevent VerifyError Created: 14/May/15  Updated: 10/Aug/15  Resolved: 20/Jul/15

Status: Closed
Project: QuickFIX/J
Component/s: Build
Affects Version/s: 1.6.0
Fix Version/s: 1.6.1

Type: Bug Priority: Major
Reporter: ManuReno Assignee: ManuReno
Resolution: Fixed Votes: 4
Labels: None

Issue Links:
Duplicate
is duplicated by QFJ-832 Multi-Version support is busted in 1.6.0 Closed
is duplicated by QFJ-858 logon problem with FIX41 Closed
Relates
is related to QFJ-855 Change packaging strategy for quickfi... Closed

 Description   

In version 1.6.0, the message-all jar is made by merging the jar each individual FIX version.
An issue arises since the type of some fields may be change between FIX versions.

For instance, the field 43 ("PossDupFlag") is a String from Fix 4.0 to Fix 4.2 and becomes a Boolean in FIX 4.3.
So classes from the jar messages-fix42 and below expect to see a StringField for field 43, whereas classes from messages-fix43 and over expect a BooleanField.
When jars are merged, only one version of the PossDupFlag is kept, resulting in a VerifyError either in fix42 and below or fix43 and over classes (depending on whioch version of PossDup has been kept).
This also happens on several other fields.

This ticket aims at changing the packaging strategy by first merging the generated source code from all FIX specs, and then compiling instead of merging classes compiled individually for each FIX spec.
The new message-all is then consistent and compatible with the quickfixj-core module which uses the same approach.

An issue remains though as only one single version of the fields that change type (field 43 for example) is compiled.
By default, the most recent version of the field is kept
It seems that this conflict was handled the same before, it thus must not be that a big issue (to be confirmed)



 Comments   
Comment by Or Ming Chun [ 18/May/15 ]

I think I have been hit by this bug. In QFJ v1.6.0, when I tried to new quickfix.fix44.NewOrderSingle(), I got:

[info] Exception encountered when attempting to run a suite with class name: com.bostontechnologies.quickfixs.messages.RichOrderSpec *** ABORTED ***
[info]   java.lang.VerifyError: Bad type on operand stack
[info] Exception Details:
[info]   Location:
[info]     quickfix/fix44/NewOrderSingle.get(Lquickfix/field/SettlType;)Lquickfix/field/SettlType; @2: invokevirtual
[info]   Reason:
[info]     Type 'quickfix/field/SettlType' (current frame, stack[1]) is not assignable to 'quickfix/CharField'
[info]   Current Frame:
[info]     bci: @2
[info]     flags: { }
[info]     locals: { 'quickfix/fix44/NewOrderSingle', 'quickfix/field/SettlType' }
[info]     stack: { 'quickfix/fix44/NewOrderSingle', 'quickfix/field/SettlType' }
[info]   Bytecode:
[info]     0x0000000: 2a2b b600 3157 2bb0

I have also tried with new quickfix.fix50.NewOrderSingle() & new quickfix.fix42.NewOrderSingle(), it works fine

Comment by dipeg [ 10/Jun/15 ]

I have the same problem, using only

Comment by dipeg [ 10/Jun/15 ]

core and messages-fix44.jar.

Comment by Or Ming Chun [ 10/Jun/15 ]

Take a look on the PR: https://github.com/quickfix-j/quickfixj/pull/29

Comment by dipeg [ 10/Jun/15 ]

Thx! Where can I take a build from?

Comment by Or Ming Chun [ 10/Jun/15 ]

if my PR merge to master soon, you can just build from the master. If not or you are in a hurry, here would help: https://github.com/mingchuno/quickfixj/tree/QFJ-845

Comment by dipeg [ 10/Jun/15 ]

Cool, I'll let you know how it goes. Thank you!

Comment by dipeg [ 10/Jun/15 ]

I built 1.7.0-SNAPSHOT from Branch QFJ-845 but the verify error is still there:

2015-06-10_20:19:45.472-ERROR-[NioProcessor-2 ]-java.lang.VerifyError: Bad type on operand stack
Exception Details:
Location:
quickfix/fix44/ExecutionReport.get(Lquickfix/field/SettlType;)Lquickfix/field/SettlType; @2: invokevirtual
Reason:
Type 'quickfix/field/SettlType' (current frame, stack[1]) is not assignable to 'quickfix/CharField'
Current Frame:
bci: @2
flags: { }
locals:

{ 'quickfix/fix44/ExecutionReport', 'quickfix/field/SettlType' }
stack: { 'quickfix/fix44/ExecutionReport', 'quickfix/field/SettlType' }

Bytecode:
0000000: 2a2b b600 5657 2bb0

{quickfix.mina.initiator.InitiatorIoHandler:85}

==> java.lang.VerifyError - Bad type on operand stack
Exception Details:
Location:
quickfix/fix44/ExecutionReport.get(Lquickfix/field/SettlType;)Lquickfix/field/SettlType; @2: invokevirtual
Reason:
Type 'quickfix/field/SettlType' (current frame, stack[1]) is not assignable to 'quickfix/CharField'
Current Frame:
bci: @2
flags: { }
locals:

{ 'quickfix/fix44/ExecutionReport', 'quickfix/field/SettlType' }
stack: { 'quickfix/fix44/ExecutionReport', 'quickfix/field/SettlType' }

Bytecode:
0000000: 2a2b b600 5657 2bb0

Comment by Chris [ 10/Jun/15 ]

@dipeg did you use the core and fix44 jars or the 'all' - I suspect the former, which means there is a clash for the SettlType class. If you use the 'all' jar, its only defined once and so should not clash.

Comment by dipeg [ 10/Jun/15 ]

@Chris, Or Min Chun: it doesn't work. Maybe I miss something but I'm pretty sure all is right now

<dependency>
<groupId>org.quickfixj</groupId>
<artifactId>quickfixj-all</artifactId>
<version>1.7.0-SNAPSHOT</version>
</dependency>

Classpath when launching the run config from intelliJ is fine, consol output confirms classpath has only quickfixj-all-1.7.0-SNAPSHOT.jar

I'd appreciate if someone could run an initiator with 1.7.0-SNAPSHOT built from https://github.com/mingchuno/quickfixj/tree/QFJ-845 receiving an ExecutionReport (I'm using FixPusher to simulate the acceptor, FIX4.4).

Thx.

Comment by Or Ming Chun [ 11/Jun/15 ]

Try to add only "org.quickfixj" % "quickfixj-core" % "1.7.0-SNAPSHOT" and "org.quickfixj" % "quickfixj-messages-all" % "1.7.0-SNAPSHOT" into your own project dependency. It should work

Comment by dipeg [ 11/Jun/15 ]

It is not working. I even deployed application to another machine to make sure that classpath is clean and only contains the jars from dependencies (see below)

  • If I change to FIX 4.2, it is OK.
  • As soon as I change to FIX 4.4: Crash with error as ==> java.lang.VerifyError - Bad type on operand stack

Could someone please confirm this behavior by sending and ExecutionReport FIX 4.4 to initiator build with QJF-854 github branch?

Thanks!

<dependency>
<groupId>org.quickfixj</groupId>
<artifactId>quickfixj-core</artifactId>
<version>1.7.0-SNAPSHOT</version>
</dependency>

<dependency>
<groupId>org.quickfixj</groupId>
<artifactId>quickfixj-messages-all</artifactId>
<version>1.7.0-SNAPSHOT</version>
</dependency>

Comment by dipeg [ 11/Jun/15 ]

This clashes with SettlType in FIX50.xml which is STRING. Looks like SettlType class from FIX50 ends up in core and then clashes

Comment by dipeg [ 11/Jun/15 ]

because in FIX44.xml, SettlType is of CHAR.

I was able to get it working by changing SettlType in FIX44.xml to STRING. Obviously, only a workaround to get the flow working ...

Comment by Chris [ 11/Jun/15 ]

Hi dipeg. It looks like you are using the core jar and messages all jar. There is a single quickfixj all jar which includes core and all messages. Thus no clash of version types. That works for me using fix4.4

Comment by dipeg [ 11/Jun/15 ]

@Chris: which quickfix version are you using?

Comment by dipeg [ 11/Jun/15 ]

@Chris: I tried quickfixj-all-1.7.0-SNAPSHOT.jar: clash for SettlType.

If I use my own, fixed FIX44.xml (CHAR -> STRING) and build a new SNAPHOT, it works with quickfixj-all-1.7.0-SNAPSHOT.jar

FIX44.xml uses SettlType CHAR
FIX50.xml uses SettlType STRING

If the wrong field class ends up in quickfixj-all-1.7.0-SNAPSHOT.jar (or core), I results in a clash. It could be that your builder packages classes for the jar in a different order so that the "correct" SettlType class makes it (coincidentally) into the final jars (!??)

Comment by Chris [ 11/Jun/15 ]

Hi @dipeg - I am using quickfixj-all version built from the above patch - but I had to add the standard FIX44/FIX50/FIXT11 xml files to the built jar.

My understanding is that SettlType will be a STRING, which is backward compatible with the CHAR version. The quickfixj-all should include the most recent version of each such class, which should be backwards compatible.

Comment by dipeg [ 12/Jun/15 ]

Hi Chris - if I used the branch, it is not working. Only if I change the xml. What do you mean by "but I had to add the standard FIX44/FIX50/FIXT11 xml files to the built jar.".?

Comment by Chris [ 13/Jun/15 ]

When I built the branch (pr/24), amongst others, I get a jar under the quickfixj-all/target directory.

This includes core and all the messages Java code - but NOT the data dictionary xml files.

Usually they are included in this jar.

So I amended the jar (via winzip) to add them in.

Comment by Chris [ 13/Jun/15 ]

Hmm, thinking about it - maybe that's the issue - I am not using the patch above, I am using this patch - https://github.com/quickfix-j/quickfixj/pull/24 ...

Comment by Nathan Tippy [ 02/Jul/15 ]

Bump, do we have an ETA on resolving the merge issues and getting this in?

Comment by Christoph John [ 03/Jul/15 ]

Sorry, did not find time to look further into this issue yet. Reading the comments I am not even sure if it is fixed or not?!

Comment by dipeg [ 03/Jul/15 ]

As you can see, I tried several things. I had only success when changing SettlType in FIX44.xml from CHAR to STRING. Obviously a hack, nothing to be considered as a serious fix.

I'm happy to further test if someone provides me with a quickfix-all jar (at a public dropbox link or whatever) built using the branch that is supposed to be merged to master.

I'm using FixPusher to simulate acceptor, FIX44 to test ExecutionReports.

Thx.

Comment by Chris [ 04/Jul/15 ]

My built file is available here - https://www.dropbox.com/sh/nt5ayl62ocn7afq/AADfmSpfMijfdvkPhWq4Z780a?dl=0 .

I added the FIX44, FIXT11 and FIX50SP2 xml files to it to then get it working with those versions - in the same JVM.

HTH, Chris

Comment by dipeg [ 06/Jul/15 ]

yes, this seems to be working, thanks Chris. However, I also had to add the FIX44.xml when UseDataDictionary=Y. All the data dictionaries (xml) are not included in your jar.

Comment by Chris [ 06/Jul/15 ]

Hi - sorry, I think the link is for the version that comes out of the maven build. The version with the dictionaries I did afterwards.

So, that makes sense that you had to add it too.

~chris

Comment by ManuReno [ 07/Jul/15 ]

Hello,
The pull request #24 has been updated to take into account the various remarks raised since the initial proposal was made (see https://github.com/quickfix-j/quickfixj/pull/24#issuecomment-119195136)
Hope it can provide a solution to the classes incompatibility issue, any feedback welcome.
Best regards

Comment by dipeg [ 07/Jul/15 ]

Hi ManuReno, I cloned your branch, built it locally, it works fine using quickfixj-all..... Thanks a lot! No SettlType class clash anymore. Would be great if someone else could do the same and send a ExecutionReport FIX44 and confirm that no SettlType issue remains.

Comment by Chris [ 07/Jul/15 ]

Thanks ManuReno - have also built your branch. Will give it a go tomorrow to check it works for me too.

Cheers.

Comment by Nathan Tippy [ 08/Jul/15 ]

I have run into a problem with the unit tests on my quad core haswell i7. It looks like a race condition.

Failed tests:
SingleThreadedEventHandlingStrategyTest.testDoubleStart:55->checkThreads:94 Exactly one 'QFJ Message Processor' thread expected expected:<1> but was:<2>

Tests run: 1294, Failures: 1, Errors: 0, Skipped: 0

I can fix the problem by adding this sleep on line 54 of SingleThreadedEventHandlingStrategyTest. However this is not the right solution but it works.

ehs.blockInThread();
Thread.sleep(200); //need delay
ehs.blockInThread();
checkThreads(bean);

Comment by Chris [ 09/Jul/15 ]

Haven't quite got all my tests working - but thats due to my issues, the quickfix stuff is all working fine - multiple versions in same process, exec reports created ok. Thanks.

Comment by Christoph John [ 09/Jul/15 ]

Nathan Tippy: yes, this is a race condition. Could you please open a separate issue for it since it has nothing to do with QFJ-845? Thanks

Comment by Nathan Tippy [ 09/Jul/15 ]

Done, that race condition issue is now QFJ-854

Generated at Thu May 02 19:36:37 UTC 2024 using JIRA 7.5.2#75007-sha1:9f5725bb824792b3230a5d8716f0c13e296a3cae.