You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2019/01/06 21:09:25 UTC

[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...

Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/guacamole-server/pull/206#discussion_r245522526
  
    --- Diff: configure.ac ---
    @@ -140,6 +140,46 @@ AC_SUBST([COMMON_SSH_INCLUDE], '-I$(top_srcdir)/src/common-ssh')
     AC_SUBST([TERMINAL_LTLIB],   '$(top_builddir)/src/terminal/libguac_terminal.la')
     AC_SUBST([TERMINAL_INCLUDE], '-I$(top_srcdir)/src/terminal $(PANGO_CFLAGS) $(PANGOCAIRO_CFLAGS) $(COMMON_INCLUDE)')
     
    +# pthread stack size
    +AC_ARG_WITH(pthread_stack,
    +            [AS_HELP_STRING([--with-pthread_stack=<size in bytes>],
    +                            [explicitly set pthread stack size (8MB is recommended)])
    +            ],pthread_stack_size=$withval
    +              AC_DEFINE_UNQUOTED([PTHREAD_STACK_SIZE], [$pthread_stack_size], [pthread stack size (8MB is recommended)])
    +)
    +if test "x$ac_cv_lib_pthread_pthread_create" = xyes; then
    +        AC_MSG_CHECKING([whether default pthread stack is larger than 8MB])
    +        ac_save_libs="$LIBS"
    +        LIBS="$LIBS -lpthread"
    +        AC_RUN_IFELSE([AC_LANG_SOURCE([[
    --- End diff --
    
    > ON THE OTHER HAND -- if you're willing to make it a firm guarantee -- not an option that this size of a stack is alway requested -- perhaps we can simplify this patch quite a bit.
    
    Making this a firm guarantee sounds like the best idea, particularly since you've demonstrated things simply don't work unless the stack size is at least 8MB. I'd suggest:
    
    1. If default stack size is less than 8MB, guacd should attempt to set the default stack size to 8MB.
    2. If the default stack size cannot be set (either because setting the attribute fails or `pthread_setattr_default_np()` is unavailable, a warning should be logged.
    
    > Is this something that would be better discussed on the mailing list?
    
    If there are other angles to consider, hopping onto the mailing list is always a good thing.


---