Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New log detection #34

Closed
alexzk1 opened this issue Jul 27, 2021 · 7 comments
Closed

New log detection #34

alexzk1 opened this issue Jul 27, 2021 · 7 comments

Comments

@alexzk1
Copy link

alexzk1 commented Jul 27, 2021

...it does not detect new game started if this program was already running (or at least proper name is not shown in status bar). I think that is because ....look on log:

{ "timestamp":"2021-07-26T19:43:11Z", "event":"Fileheader", "part":1, "language":"English\UK", "Odyssey":true, "gameversion":"4.0.0.600", "build":"r271793/r0 " }
{ "timestamp":"2021-07-26T19:44:15Z", "event":"Friends", "Status":"Online", "Name":"" }
{ "timestamp":"2021-07-26T19:47:42Z", "event":"Commander", "FID":"---", "Name":"---" }

On time-stamps.
It is 4 seconds difference since 1st line and commander name happens in last line.
However, this handler will be called on 1st line - once new file created - at best case, and that complex IF will reject event.

public void onCreated(final FileEvent event) {
                final File file = event.getFile();
                if (file.isFile()) {
                    if (file.getName().startsWith(AppConstants.JOURNAL_FILE_PREFIX) && hasFileHeader(file) && isOdysseyJournal(file) && hasCommanderHeader(file) && isSelectedCommander(file)) {
                        JournalWatcher.this.watchedFile = Optional.of(file);
                        fileCreatedProcessor.accept(file);
                    }
                }
            }

The event " public void onModified(final FileEvent event)" seems the same, except it calls different handler...didn't dig there too much.

Also FileProcessor.processJournal must be called from 1 thread with current implementation. Theoretically if game will restart fast and new file will be detected while that thing is running - results will be weird. However not possible to make in practice, you can try to break things by copying log files to folder while program runs.

@jixxed
Copy link
Owner

jixxed commented Jul 27, 2021

They used to be different, but now only onModified is needed. need to do some work on this class indeed. Statusbar shows latest file IF a message of it was processed, but that should be possible to do sooner.
I should make it threadsafe even if it never happens in practice, don't know how this class will change in the future.

@alexzk1
Copy link
Author

alexzk1 commented Jul 28, 2021

Make this
FileWatcher::
boolean poll = true;
to be AtomicBoolean
Because it writes in 2 places in stop() and during poll() which are different threads. Also inside thread, inside while, you must atomically read old + set new if old remains true, because while method poll() executes it could be called stop() which will set it false, but that false will be lost.
Atomics allow to do read-set-check as single operation, guaranteed values will not be changed in between.

This is double problem here.
boolean poll = true;
Maybe cached on different processors so value of that will be not the same when 2 CPUs read it. To solve it you could use keyword volatile.
Then 2nd problem - stop() writes false and poll() writes true -- false is lost. To solve it you could use synchronize keyword + dedicated Object for that.
However, using atomics you may avoid both problems at once without delays for sync.

@jixxed
Copy link
Owner

jixxed commented Jul 28, 2021

Made some of the changes in the 1.34 release. Should be stable enough for now, but will investigate this a bit more the coming days. Bit hard to really focus now after my 2nd vaccination. Main features are in now, so time for some refactoring and testing.

@alexzk1
Copy link
Author

alexzk1 commented Jul 28, 2021

Made some of the changes in the 1.34 release. Should be stable enough for now, but will investigate this a bit more the coming days. Bit hard to really focus now after my 2nd vaccination. Main features are in now, so time for some refactoring and testing.

Yes, np. Just saying that code has fundamental error. Because that will be doing random errors at random times, I explained why. In short - you can't use normal variables if it is accessed by 2+ threads.

@alexzk1
Copy link
Author

alexzk1 commented Jul 29, 2021

You're getting close, proper would be

while (this.poll.getAndSet(pollEvents(watchService));

that will ensure if stop called prior pollEvents, it's false result will be noted.

P.S. don't forget to revise all threads u do there (if any).

P.P.S. They lowered Kit Flowler from 20 to 10 for unlock today.

@alexzk1
Copy link
Author

alexzk1 commented Jul 29, 2021

Yep, splitting into 2 vars was best solution.
private boolean poll = true;
now this can be local variable, to minimize side effects...and C++ style:

for (boolean poll = true;poll && this.run.get(); poll = pollEvents(watchService)) ;

@jixxed
Copy link
Owner

jixxed commented Jul 30, 2021

threading has been reworked

@jixxed jixxed closed this as completed Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants