You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by shinrich <gi...@git.apache.org> on 2017/03/10 16:00:55 UTC
[GitHub] trafficserver pull request #1565: Fix Assertion failure in the regex_revalid...
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/1565
Fix Assertion failure in the regex_revalidate plugin.
Since TS-4387, Calls to TSContSchedule/TSContScheduleEvery(), require
that the continuation associated with the TSCont parameter must have a mutex.
(cherry picked from commit 0b1f28b53174baf5cfff54a2d224ffbe09a64374)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver issue-1561
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1565.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 #1565
----
commit a1755d6cc2bb43547962b0e4e4f61d62a2243150
Author: John J. Rushford <jr...@apache.org>
Date: 2017-02-01T20:34:44Z
Fix Assertion failure in the regex_revalidate plugin.
Since TS-4387, Calls to TSContSchedule/TSContScheduleEvery(), require
that the continuation associated with the TSCont parameter must have a mutex.
(cherry picked from commit 0b1f28b53174baf5cfff54a2d224ffbe09a64374)
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/29/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1565
@jpeach I'll take care of updating the docs in a direct commit, it's not worth a PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/155/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/30/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/1565
Merged to 7.1.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:
https://github.com/apache/trafficserver/pull/1565
@amc - thanks, I just saw @jpeach comment.
@zwoop - Yes this change fixes the issue. We ran into the problem running 6.2.x in production. The PR that mad a mutex a requirement was from this Jira: https://issues.apache.org/jira/browse/TS-4387?jql=text%20~%20%22TSContSchedule%22
regex_revalidate may have been overlooked when they updated plugins using TSContSchedule()
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1565: Fix Assertion failure in the regex_revalid...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1565#discussion_r106033633
--- Diff: plugins/regex_revalidate/regex_revalidate.c ---
@@ -362,7 +362,7 @@ config_handler(TSCont cont, TSEvent event ATS_UNUSED, void *edata ATS_UNUSED)
iptr = __sync_val_compare_and_swap(&(pstate->invalidate_list), pstate->invalidate_list, i);
if (iptr) {
- free_cont = TSContCreate(free_handler, NULL);
+ free_cont = TSContCreate(free_handler, TSMutexCreate());
--- End diff --
I'm ok with this (assuming this solves the problem, it's not immediately clear as to why it needs the mutex). However, this approach of a schedule "delete" seems bad, we should change this (later) to use ref-counted data instead. That would eliminate the entire free_cont, and is a better / more reliable pattern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/287/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/43/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
FreeBSD11 build *successful*! https://ci.trafficserver.apache.org/job/freebsd-github/1725/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1565: Fix Assertion failure in the regex_revalid...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop closed the pull request at:
https://github.com/apache/trafficserver/pull/1565
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/1565
Consider updating the API documentation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by jrushford <gi...@git.apache.org>.
Github user jrushford commented on the issue:
https://github.com/apache/trafficserver/pull/1565
@zwoop there was a change, https://issues.apache.org/jira/browse/TS-4387, made that requires a mutex when using TSContSchedule(). With the PR, all plugins using TSContSchedule were updated to supply a mutex on the continuation scheduled by TSContSchedule(). Sdding the Mutex() did solve the assertion failures in 6..2 that we saw whenever the regex_revalidate config file was changed.
Thanks
--
John J. Rushford
From: Leif Hedstrom <no...@github.com>
Reply-To: apache/trafficserver <re...@reply.github.com>
Date: Tuesday, March 14, 2017 at 3:36 PM
To: apache/trafficserver <tr...@noreply.github.com>
Cc: John Rushford <Jo...@cable.comcast.com>, Review requested <re...@noreply.github.com>
Subject: Re: [apache/trafficserver] Fix Assertion failure in the regex_revalidate plugin. (#1565)
@zwoop commented on this pull request.
________________________________
In plugins/regex_revalidate/regex_revalidate.c<https://github.com/apache/trafficserver/pull/1565#discussion_r106033633>:
> @@ -362,7 +362,7 @@ config_handler(TSCont cont, TSEvent event ATS_UNUSED, void *edata ATS_UNUSED)
iptr = __sync_val_compare_and_swap(&(pstate->invalidate_list), pstate->invalidate_list, i);
if (iptr) {
- free_cont = TSContCreate(free_handler, NULL);
+ free_cont = TSContCreate(free_handler, TSMutexCreate());
I'm ok with this (assuming this solves the problem, it's not immediately clear as to why it needs the mutex). However, this approach of a schedule "delete" seems bad, we should change this (later) to use ref-counted data instead. That would eliminate the entire free_cont, and is a better / more reliable pattern.
\u2014
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub<https://github.com/apache/trafficserver/pull/1565#pullrequestreview-26939009>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEVybl5q-yr1BXSWc-Z9QeVRgIsqo1PSks5rlwhkgaJpZM4MZjXB>.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1565: Fix Assertion failure in the regex_revalidate plu...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1565
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1620/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---