Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

simplify sigatom code #212

Merged
merged 3 commits into from
May 11, 2016
Merged

simplify sigatom code #212

merged 3 commits into from
May 11, 2016

Conversation

vtjnash
Copy link
Contributor

@vtjnash vtjnash commented Dec 27, 2015

this doesn't fix 161, it just moves the failure condition so that it
should be much less common and more avoidable

gtk_yielded = false # when true, use the `gtk_doatomic` queue to run sigatom functions
gtk_doatomic = [] # (work, notification) scheduler queue
g_sigatom_flag = false # keep track of Base sigatomic state
function g_sigatom(f::Base.Callable) # calls f, were f never throws (but this function may throw)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

were->where

this doesn't fix 161, it just moves the failure condition so that it
should be much less common and avoidable

also fixes performance issue with compiling finalizers (esp. at-exit)
in v0.4, these were anonymous functions
in v0.5, now need to declare the argument type as ::ANY
to prevent specializations
@vtjnash vtjnash changed the title WIP: simplify sigatom code simplify sigatom code May 10, 2016
@vtjnash
Copy link
Contributor Author

vtjnash commented May 10, 2016

This should be better now and make #161 happier

@vtjnash
Copy link
Contributor Author

vtjnash commented May 10, 2016

So, even though this PR doesn't fix #161, it makes it more trivial to work around:

  1. if you are in a callback, don't yield. use @async print, etc, or queue a message for yourself, etc. You can also call GLib.g_yield(), but see 2
  2. if you are getting segfaults in some random method due to there existing a callback that recursively calls the glib main loop (such as making a dialog box) or otherwise causes g_yield to be called, wrap the faulting code in @sigatom. This will postpone execution of the code until it can be run on the proper stack (but otherwise acts like normal control flow).

I think these might be places that it would try to access the Gtk stac
stk = gtk_stack
g_stack = nothing # need to call g_loop_run from only one stack
const g_yielded = Ref(false) # when true, use the `g_doatomic` queue to run sigatom functions
const g_doatomic = [] # (work, notification) scheduler queue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any virtue in declaring this Tuple{Any,Any}[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that would be slower

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine then.

@timholy
Copy link
Member

timholy commented May 10, 2016

Maybe add a version of those two comments to the README?

💯

@vtjnash vtjnash merged commit a96040b into master May 11, 2016
@vtjnash vtjnash deleted the jn/161 branch May 11, 2016 16:05
@timholy
Copy link
Member

timholy commented May 11, 2016

Awesome. Tag a new release?

@vtjnash
Copy link
Contributor Author

vtjnash commented May 12, 2016

I figured I would give it a day or two for the bug reports to start coming in. But if it seems to be working, then sure, I can tag a new release.

@timholy
Copy link
Member

timholy commented May 12, 2016

I haven't tested it yet myself, but that may change by the end of the day.

Thanks for tackling this, I'm really looking forward to it.

@timholy
Copy link
Member

timholy commented May 12, 2016

I tested whether I could live without JuliaGtk/GtkUtilities.jl#7 by wrapping
https://github.com/timholy/GtkUtilities.jl/blob/9ef4557c7531975ccde2f7e245596588bcfc939c/src/guidata.jl#L42-L48 in @sigatom and/or @schedule. It's much better, but I still can get the segfault.

If you need some test code, I can arrange something. That said, I know you have many priorities, so no rush.

@vtjnash
Copy link
Contributor Author

vtjnash commented May 12, 2016

Test code would be good. There shouldn't be any difference between guarded and siginterruptable now

@tknopp
Copy link
Collaborator

tknopp commented May 20, 2016

@vtjnash: I have tested it during last week and sigatom made all my segfaulting apps just work. This is very much appreciated. Would be great if a new release could be tagged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants