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

Clean shutdown of DirPoll when file processing is in progress #38

Merged
merged 7 commits into from
Feb 17, 2014

Conversation

vishnupillai
Copy link
Contributor

DirPollAdaptor does not wait for the dirPoll thread to finish during shutdown. This can cause a dirty shutdown if the dirPoll thread were in the midst of processing a file.
This change makes the shutdown wait for the dirPoll thread to exit in a graceful manner to good extent and then interrupts the thread if still running

@vishnupillai
Copy link
Contributor Author

@ar Please hold. I'm checking why CI failed.

@vishnupillai
Copy link
Contributor Author

@ar I've changed a test. Please review and let me know if that causes any issues

@ar
Copy link
Member

ar commented Feb 17, 2014

Vishnu,

Thanks for the PR, but this PR has a merge that could have been rebased, and I'm afraid we'd loose history. In addition, there's a lot of whitespace changes that I'd appreciate if we could make place it in a separate PR or at least a separate commit, it's difficult to tell apart whitespace changes versus the real changed code.

Can you rebase master and re-apply just the DirPoll code fix and its test ?

@ar
Copy link
Member

ar commented Feb 17, 2014

Forget my comment, I can fix this locally.

@ar ar merged commit 7dd4090 into jpos:master Feb 17, 2014
@vishnupillai
Copy link
Contributor Author

@ar Thank you. My editor put in some whitespace changes and I tried to revert all the whitespace changes with another commit. Sorry for the multiple commits. The changes may be easier to see if you look at https://github.com/jpos/jPOS/pull/38/files.

@ar
Copy link
Member

ar commented Feb 17, 2014

Exactly, and they installed cleanly when I pulled the pr locally. Sorry for my initial concern, thought a lot more things were changed.

@vishnupillai
Copy link
Contributor Author

Thanks Alejandro. My over-enthusiasm let to the multiple commits which is not really good. I'm still learning :)

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

Successfully merging this pull request may close these issues.

2 participants