[GitHub] guacamole-server pull request #207: GUACAMOLE-673: Create automatically need...
GitHub user dejan032 opened a pull request: https://github.com/apache/guacamole-server/pull/207 GUACAMOLE-673: Create automatically needed directory for file transfer. Automatically create transfer folder in guacd file system when user is logged in to rdp, so that the transfer network drive can be used immediately without needing to open it first one time to start copying things into and out of it. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dejan032/guacamole-server master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/guacamole-server/pull/207.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #207 commit 48974cf79fa629dc63d35665d57cfdc79dd8fda4 Author: Dejan Milovanovic Date: 2018-12-04T07:31:45Z GUACAMOLE-673: Create automatically needed directory for file transfer. ---
[GitHub] guacamole-server pull request #204: GUACAMOLE-354: Add Swiss-German keymap f...
Github user andrin-jost commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/204#discussion_r238549377 --- Diff: src/protocols/rdp/keymaps/de_ch_qwertz.keymap --- @@ -0,0 +1,58 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +parent "base" +name"ch-de-qwertz" --- End diff -- Oh, yes you're right ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238508851 --- 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=], +[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 -- I honestly don't know if there is. I couldn't come up with one. That said -- this is basically a runtime check for a warning message. As such I can remove it (while leaving the rest of the patch). It would, however, be *very* unfortunate to do so since in my personal example it cost me a day to figure out that it was the stack size that was killing guacd on alpine. Anybody building guacd in the future on a similar platform will surely appreciate getting this type of a message early. 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. Is this something that would be better discussed on the mailing list? ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507952 --- 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=], +[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 --- End diff -- See the comment above. It is automagically created by autoconf because of the test you quoted (testing for pthread_create in a library called pthread). ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507999 --- 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=], +[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([[ +#include + +int main() { +pthread_attr_t default_pthread_attr; +size_t stacksize; + +#ifdef PTHREAD_STACK_SIZE +return PTHREAD_STACK_SIZE < 8*1024*1024; +#else +pthread_attr_init(&default_pthread_attr); +pthread_attr_getstacksize(&default_pthread_attr, &stacksize); +return stacksize < 8*1024*1024; +#endif +} + + ]])], + [AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no]) + AC_MSG_WARN([ + + Default pthread stack size is less than 8MB --- End diff -- Yes we do. This is the default with glibc. ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507820 --- 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=], +[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" --- End diff -- I'm not sure I follow this comment -- this is exactly why I'm using. ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507693 --- 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=], +[explicitly set pthread stack size (8MB is recommended)]) --- End diff -- Would you prefer it to read "8388608 (8MB)" ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507594 --- Diff: src/guacd/daemon.c --- @@ -275,6 +281,17 @@ int main(int argc, char* argv[]) { /* General */ int retval; +#ifdef PTHREAD_STACK_SIZE +/* Set default stack size */ +pthread_attr_t default_pthread_attr; +if (pthread_attr_init(&default_pthread_attr) || +pthread_attr_setstacksize(&default_pthread_attr, PTHREAD_STACK_SIZE) || +pthread_setattr_default_np(&default_pthread_attr)) { + fprintf(stderr, "Couldn't set requested pthread stack size of %d\n", PTHREAD_STACK_SIZE); + exit(EXIT_FAILURE); --- End diff -- Got it! Will fix. ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507556 --- Diff: src/guacd/daemon.c --- @@ -19,6 +19,12 @@ #include "config.h" +#ifdef PTHREAD_STACK_SIZE +#define _GNU_SOURCE 1 --- End diff -- pthread_setattr_default_np() is available for quite sometime with _GNU_SOURCE so I don't think this should be that big of a concern. To make it reliable I can add testing for pthread_setattr_default_np to the configure part of this patch. Consider this done. As for the more portable way of doing this -- the only way I know of is to add add explicit setting of the stack size right before each call of pthread_create. Please let me know if you'd prefer this way of doing it (I can see myself wrapping it up in a single function of course). ---
[GitHub] guacamole-server pull request #206: GUACAMOLE-663: guacd SEGVs intermittentl...
Github user rvs commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/206#discussion_r238507157 --- Diff: src/guacd/daemon.c --- @@ -275,6 +281,17 @@ int main(int argc, char* argv[]) { /* General */ int retval; +#ifdef PTHREAD_STACK_SIZE +/* Set default stack size */ +pthread_attr_t default_pthread_attr; +if (pthread_attr_init(&default_pthread_attr) || +pthread_attr_setstacksize(&default_pthread_attr, PTHREAD_STACK_SIZE) || +pthread_setattr_default_np(&default_pthread_attr)) { + fprintf(stderr, "Couldn't set requested pthread stack size of %d\n", PTHREAD_STACK_SIZE); --- End diff -- Good point. Will fix both of these. ---
Re: Help with CAS Module Regression
On Mon, Dec 3, 2018 at 10:51 AM Nick Couchman wrote: > On Mon, Dec 3, 2018 at 10:49 AM Mike Jumper wrote: > >> Did this error occur in 0.9.14? >> >> If this is breaking things, and did not occur when CAS support was >> initially released, I'd suggest a git bisect to narrow down when it >> started. >> >> > I believe everything worked fine in 0.9.14, and that something in the > changes between there and staging/1.0.0 has broken things. I'll work on a > git bisect of the changes and see if I can track it down. > > Looks like the change to AngularJS 1.6.9 broke something:
Re: Help with CAS Module Regression
On Mon, Dec 3, 2018 at 3:57 PM Nick Couchman wrote: > On Mon, Dec 3, 2018 at 10:51 AM Nick Couchman wrote: > >> On Mon, Dec 3, 2018 at 10:49 AM Mike Jumper wrote: >> >>> Did this error occur in 0.9.14? >>> >>> If this is breaking things, and did not occur when CAS support was >>> initially released, I'd suggest a git bisect to narrow down when it >>> started. >>> >>> >> I believe everything worked fine in 0.9.14, and that something in the >> changes between there and staging/1.0.0 has broken things. I'll work on a >> git bisect of the changes and see if I can track it down. >> >> > Looks like the change to AngularJS 1.6.9 broke something: > b3eeb36b8726211597bdc921a632dbbc5c87ee16 is the first bad commit commit b3eeb36b8726211597bdc921a632dbbc5c87ee16 Author: James Muehlner Date: Wed Apr 25 22:25:02 2018 -0700 GUACAMOLE-526: Update webapp to angular 1.6.9. :04 04 461bb123891a0c463bd1fbf8c84fbd3070633d09 11d67328a11952849a126e88fb68e6895d5f92b0 M guacamole
Re: Help with CAS Module Regression
On Mon, Dec 3, 2018 at 10:49 AM Mike Jumper wrote: > Did this error occur in 0.9.14? > > If this is breaking things, and did not occur when CAS support was > initially released, I'd suggest a git bisect to narrow down when it > started. > > I believe everything worked fine in 0.9.14, and that something in the changes between there and staging/1.0.0 has broken things. I'll work on a git bisect of the changes and see if I can track it down. -Nick
Re: Help with CAS Module Regression
On Sun, Dec 2, 2018 at 1:48 PM Nick Couchman wrote: > > Okay, banging my head against the wall here trying to figure out what's > going on with the CAS module. At this point the redirect doesn't work at > all - the CAS module just sits on the screen with the non-translated > message: > > {{ 'LOGIN.INFO_CAS_REDIRECT_PENDING' | translate }} > > and never redirects. In the JS console I get the following: > > Failed to retrieve field with type "GUAC_CAS_TICKET" > That specific message will occur if a custom field type cannot be used: https://github.com/apache/guacamole-client/blob/fc457c080d813044e30e1f4e8fe855d6a5900259/guacamole/src/main/webapp/app/form/directives/formField.js#L152 That's just a generic "catch", though. If an exception occurs somewhere within the promise handling (due to something now being broken in our handling of custom fields?), you could see the same result, despite the field type being correctly registered. If this is the case, other things that use custom fields like Duo or TOTP may also not work as expected. Worth rechecking. > Any ideas what might be going on here? I've looked at the various parts of > the CAS module, particularly the AngularJS stuff, and compared to OpenID to > see what differences might be there, and can't see anything obvious. I'm > sure it's something simple/stupid, but I need another set of eyes on it. > Also, has the OpenID module been tested recently to make sure it works? I > don't think I have an OpenID provider I can test against, else I would do > it myself... I have a test deployment which uses OpenID against Google - I'll recheck. > > Let me know if I can provide any additional information. I don't see > anything obviously wrong in the catalina.out file, so figure it's something > AngularJS-based that's going on. > Did this error occur in 0.9.14? If this is breaking things, and did not occur when CAS support was initially released, I'd suggest a git bisect to narrow down when it started. - Mike
[GitHub] guacamole-server pull request #204: GUACAMOLE-354: Add Swiss-German keymap f...
Github user mike-jumper commented on a diff in the pull request: https://github.com/apache/guacamole-server/pull/204#discussion_r238316700 --- Diff: src/protocols/rdp/keymaps/de_ch_qwertz.keymap --- @@ -0,0 +1,58 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +parent "base" +name"ch-de-qwertz" --- End diff -- The keyboard name should be LANG-COUNTRY-VARIANT, same as the filename except with dashes instead of underscores. Is "de-ch-qwertz" correct here or "ch-de-qwertz"? ---
[GitHub] guacamole-client pull request #341: GUACAMOLE-670: Add slf4j logging to CAS ...
Github user asfgit closed the pull request at: https://github.com/apache/guacamole-client/pull/341 ---
[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_r238167956 --- Diff: src/guacd/daemon.c --- @@ -275,6 +281,17 @@ int main(int argc, char* argv[]) { /* General */ int retval; +#ifdef PTHREAD_STACK_SIZE +/* Set default stack size */ +pthread_attr_t default_pthread_attr; +if (pthread_attr_init(&default_pthread_attr) || +pthread_attr_setstacksize(&default_pthread_attr, PTHREAD_STACK_SIZE) || +pthread_setattr_default_np(&default_pthread_attr)) { + fprintf(stderr, "Couldn't set requested pthread stack size of %d\n", PTHREAD_STACK_SIZE); --- End diff -- Why `fprintf()` early rather than `guacd_log()` after the logging system is ready? ---
[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_r238170624 --- Diff: src/guacd/daemon.c --- @@ -19,6 +19,12 @@ #include "config.h" +#ifdef PTHREAD_STACK_SIZE +#define _GNU_SOURCE 1 --- End diff -- I'm concerned over explicitly setting `_GNU_SOURCE` and using the non-portable `pthread_setattr_default_np()`. There's no guarantee that `pthread_setattr_default_np()` will be available, or that it will be available if `_GNU_SOURCE` is set. Testing for these within configure would be an improvement, but the non-portability is still concerning. Is there no portable way to do this? ---
[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_r238171069 --- 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=], +[explicitly set pthread stack size (8MB is recommended)]) --- End diff -- This may be confusing as "8MB" is not a value accepted by this option. ---
[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_r238171680 --- 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=], +[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" --- End diff -- Not all platforms use libpthread for POSIX threads. Some have this built into libc. If necessary, you might be able to use `$PTHREAD_LIBS` which is set by an earlier test: https://github.com/apache/guacamole-server/blob/bbb6afaf462f6930a589f962302806c52f350c0b/configure.ac#L61-L64 ---
[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_r238170864 --- Diff: src/guacd/daemon.c --- @@ -275,6 +281,17 @@ int main(int argc, char* argv[]) { /* General */ int retval; +#ifdef PTHREAD_STACK_SIZE +/* Set default stack size */ +pthread_attr_t default_pthread_attr; +if (pthread_attr_init(&default_pthread_attr) || +pthread_attr_setstacksize(&default_pthread_attr, PTHREAD_STACK_SIZE) || +pthread_setattr_default_np(&default_pthread_attr)) { + fprintf(stderr, "Couldn't set requested pthread stack size of %d\n", PTHREAD_STACK_SIZE); + exit(EXIT_FAILURE); --- End diff -- Please use 4-space indents per level. See: http://guacamole.apache.org/guac-style/#general-style ---
[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_r238172953 --- 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=], +[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 -- [The documentation for `AC_RUN_IFELSE`](https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Runtime.html) states: > Avoid running test programs if possible, because this prevents people from configuring your package for cross-compiling. Is there any other way to achieve this which would not hurt cross-compilation? ---
[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_r238168882 --- Diff: src/guacd/daemon.c --- @@ -275,6 +281,17 @@ int main(int argc, char* argv[]) { /* General */ int retval; +#ifdef PTHREAD_STACK_SIZE +/* Set default stack size */ +pthread_attr_t default_pthread_attr; +if (pthread_attr_init(&default_pthread_attr) || +pthread_attr_setstacksize(&default_pthread_attr, PTHREAD_STACK_SIZE) || +pthread_setattr_default_np(&default_pthread_attr)) { + fprintf(stderr, "Couldn't set requested pthread stack size of %d\n", PTHREAD_STACK_SIZE); --- End diff -- > `"Couldn't set requested pthread stack size of %d\n"` At the time this failure occurs, the user hasn't requested anything - the stack size is set permanently at build time. This message should be rephrased such that the nature of the error can be understood by the user running guacd. ---
[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_r238172374 --- 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=], +[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([[ +#include + +int main() { +pthread_attr_t default_pthread_attr; +size_t stacksize; + +#ifdef PTHREAD_STACK_SIZE +return PTHREAD_STACK_SIZE < 8*1024*1024; +#else +pthread_attr_init(&default_pthread_attr); +pthread_attr_getstacksize(&default_pthread_attr, &stacksize); +return stacksize < 8*1024*1024; +#endif +} + + ]])], + [AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no]) + AC_MSG_WARN([ + + Default pthread stack size is less than 8MB --- End diff -- Do we know for certain that 8MB satisfies our stack requirements? ---
[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_r238171724 --- 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=], +[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 --- End diff -- Where does `ac_cv_lib_pthread_pthread_create` come from? ---