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

controller & notify events queue code improvements #35

Closed
wants to merge 35 commits into from
Closed

controller & notify events queue code improvements #35

wants to merge 35 commits into from

Conversation

wulf7
Copy link
Contributor

@wulf7 wulf7 commented Feb 27, 2016

  1. fix signal handling issue https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=204366
  2. Some controller fixes and improvements. Stress test is not included as it is unportable now :-(
  3. inotify events queue code is completed so https://github.com/idea4bsd/fsnotifier/issues/1 is fixed

Asyncronous signal handlers should not be called from worker thread
context to not interfere with main application. See [1] for example.
Moreover libinotify in current state do not use any syncronous signals.
So we can block em all not only SIGPIPE

[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=204366
This allows worker thread not wait for user thread so its possible
to unconditionaly call signaling function at watch closing
as it makes user<->worker interactions harder to understand
Compare io[INOTIFY_FD] with -1 instead of
to allow parallel command execution on different workers.
Commands for the same worker are serialized with worker`s own mutex
All threads of stress test run in parallel now.
This allows to avoid executing commands in wrong workers while
stress-testing caused by quick reopen of inotify instances with
the same id
This should prevent blocking of worker thread on socket write caused
by send buffer overflow if no readers exists. It can happen e.g.
on mass-removing of watches when lot of IN_IGNORED events are generated
but noone need them.

https://github.com/idea4bsd/fsnotifier/issues/1
That should prevent blocking of working thread on writting of inotify
events to user`s socket when amount of data to be written exceedes free
socket buffer space
@wulf7 wulf7 mentioned this pull request Feb 27, 2016
or with SO_NOSIGPIPE socket option. Hopefully, this fixes [1] on all
modern BSDs

[1]. #8
It seems not all OSes support -1 as GID number
@wulf7
Copy link
Contributor Author

wulf7 commented Mar 6, 2016

Hmmm. Some development code leaked (last 3 commits). So I overwrote it with up to date versions with forced push to not spoil the commit history

@dmatveev
Copy link
Owner

dmatveev commented Mar 9, 2016

So many changes, thanks Vladimir, will definitely merge it soon!

wulf7 added 8 commits May 1, 2016 13:42
Closing of duped file descriptors should not produce any notifications
until close of last descriptor referenced a file
Improved version of it has already been commited to FreeBSD source tree
as r298982. Improvements are:

1. Split NOTE_CLOSE on NOTE_CLOSE and NOTE_CLOSE_WRITE events to be able
   to separate readers from writers and match Linux inotify interface.
2. Emit NOTE_READ on readdir operation
3. Dont send NOTE_CLOSE from vgonel() on device unmounting

https://svnweb.freebsd.org/base?view=revision&revision=298982
@wulf7
Copy link
Contributor Author

wulf7 commented May 4, 2016

Hi, Dmitry.
A there any news on these changes?
Given the fact that bugfixes for 2 critical bugs affecting at least users of inotifywatch and intelijdea are not even reviewed within 2 month I consider this project to be unsupported. And Im really tired of your inability to support this so it would better if you would just write that "you cant do it" somewhere on your page to not confuse people using the project
Iam also have private life and very busy on BSD-unrelated work and nobody pays me for libinotify so I have to spent my priceless free time trying to develop and support it and I'm not fond of the fact that my efforts go to the thrash can. Im very sorry if my words insults you but I think now is the time to do something with a current state of things.

@dmatveev dmatveev self-assigned this May 4, 2016
@dmatveev
Copy link
Owner

dmatveev commented May 4, 2016

Hi Vladimir,

Sorry for a delay. Your efforts will never go to the thrash can!

Here is what I can do. Please push all your work here: https://github.com/libinotify-kqueue/libinotify-kqueue

I will put a notice in my repository that it is not "official" anymore.

Dmitry

@wulf7
Copy link
Contributor Author

wulf7 commented May 5, 2016

Thank you. I have pushed master branch here

@wulf7 wulf7 closed this May 5, 2016
@wulf7
Copy link
Contributor Author

wulf7 commented May 5, 2016

master repository has been moved to https://github.com/libinotify-kqueue/libinotify-kqueue

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