[QFJ-923] FileStore is leaking file handles Created: 20/Apr/17 Updated: 27/Jul/17 Resolved: 19/Jul/17 |
|
Status: | Closed |
Project: | QuickFIX/J |
Component/s: | Engine |
Affects Version/s: | 1.6.2, 1.6.3 |
Fix Version/s: | 1.6.4 |
Type: | Bug | Priority: | Critical |
Reporter: | Constantin Florescu | Assignee: | Christoph John |
Resolution: | Fixed | Votes: | 0 |
Labels: | Reconnect, logon, timeout | ||
Environment: |
All |
Issue Links: |
|
Description |
The FileStore like all the other stores are not synchronized. When a session is about to be estabilished, logon request is sent to the server, if the request is not answered by the server in within the timeout period the connection is closed and the reconnect timer starts. The problem is that when the Timeout Timer [QFJ Timer] fires it also releases [QFJ Message Processor] thread which tries to process EndOfFille/Stream event, both threads will try to reset the MessageStore on next(). During the reset() the FileStore is (re)initialized which causes the following actions: close existing files, open the files, read stores. When done in parallel, one thread may try to open or read while the other is closing the files resulting in: Some proof: And from: Out of file handles: |
Comments |
Comment by Constantin Florescu [ 20/Apr/17 ] |
It is related to: |
Comment by Christoph John [ 21/Apr/17 ] |
Hi, I'm wondering if this specific issue (leaking file handles) can be fixed like |
Comment by Constantin Florescu [ 24/Apr/17 ] |
Unfortunately The synchronized function reset() in class Session is not called in my case. (See stack traces) |
Comment by Christoph John [ 24/Apr/17 ] |
I know that |
Comment by Constantin Florescu [ 24/Apr/17 ] |
Sorry, I misunderstood you. "can be fixed like The following updated resetState() method fixes my problem. I just hope there are no other places where the "Stores" are accessed concurrently. private void resetState() { if (!isResettingState.compareAndSet(false, true)) { return; } try { state.reset(); stateListener.onReset(); } finally { isResettingState.set(false); } } |
Comment by Christoph John [ 24/Apr/17 ] |
OK, I meant literally the keyword "synchronize" |
Comment by Constantin Florescu [ 24/Apr/17 ] |
For me it is not urgent because I have a work around. About the aforementioned pull request #75, it contains only one file, FileLog, however the issue is in FileStore (MessageStore implementors in general). Is this the right pull request? |
Comment by Christoph John [ 24/Apr/17 ] |
Sorry, you are completely right. I did not remember correctly that the pull request is only about the FileLog. |
Comment by Constantin Florescu [ 24/Apr/17 ] |
No. It is just synchronizing the resetState as above. The downside is that I do not know if that is the only concurrency issue when accessing a MessageStore. |