[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 A different failure this time: ``` --- FAIL: TestHttpContextTimeout (0.00s) context_test.go:74: Unexpected error: dial tcp 127.0.0.1:9096: getsockopt: connection refused ``` ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350182#comment-16350182 ] ASF GitHub Bot commented on THRIFT-4448: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 A different failure this time: ``` --- FAIL: TestHttpContextTimeout (0.00s) context_test.go:74: Unexpected error: dial tcp 127.0.0.1:9096: getsockopt: connection refused ``` > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-760) Generated client code does not set or check the sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-760?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350173#comment-16350173 ] Jacquet Fabian commented on THRIFT-760: --- hello all, I know this defect is closed but it has not been fixed for c++. I have a patch for c++, how can I provide it? > Generated client code does not set or check the sequence ID in messages > --- > > Key: THRIFT-760 > URL: https://issues.apache.org/jira/browse/THRIFT-760 > Project: Thrift > Issue Type: Bug > Components: Java - Compiler >Affects Versions: 0.2 >Reporter: James Grant >Assignee: James Grant >Priority: Minor > Fix For: 0.3 > > Attachments: ASF.LICENSE.NOT.GRANTED--thrift-seqid.patch > > > The sequence ID is never set in the generated client code. It is also never > checked. This means that if you continue to use a connection after a socket > timeout the returned results can arrive out of sequence. When this happens an > exception should be thrown so that incorrect results are never returned. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift pull request #1490: THRIFT-4454 Large writes/reads may cause range ch...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1490 ---
[GitHub] thrift pull request #1489: THRIFT-4485 Possible invalid ptr AV with overlapp...
Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1489 ---
[jira] [Commented] (THRIFT-4485) Possible invalid ptr AV with overlapped read/write on pipes
[ https://issues.apache.org/jira/browse/THRIFT-4485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350307#comment-16350307 ] ASF GitHub Bot commented on THRIFT-4485: Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1489 > Possible invalid ptr AV with overlapped read/write on pipes > --- > > Key: THRIFT-4485 > URL: https://issues.apache.org/jira/browse/THRIFT-4485 > Project: Thrift > Issue Type: Bug > Components: Delphi - Library >Reporter: Jens Geyer >Assignee: Jens Geyer >Priority: Critical > > If a read or write operation on pipes reaches the set timeout, the read/write > operation is not properly cancelled. However, the overlapped struct gets > freed when leaving the method, which essentially leaves the pending read or > write operation with an undefined pointer. > Easily reproducible with buffered transport over pipes, a combination that > does not work at all anyways. The workaround for both problems is to not use > buffered transport with pipes (use framed instead), and some sane tinemouts > (not too short). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-4454) Large writes/reads may cause range check errors in debug mode
[ https://issues.apache.org/jira/browse/THRIFT-4454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350306#comment-16350306 ] ASF GitHub Bot commented on THRIFT-4454: Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/1490 > Large writes/reads may cause range check errors in debug mode > - > > Key: THRIFT-4454 > URL: https://issues.apache.org/jira/browse/THRIFT-4454 > Project: Thrift > Issue Type: Bug > Components: Delphi - Library >Affects Versions: 0.11.0 >Reporter: Jens Geyer >Assignee: Jens Geyer >Priority: Major > Fix For: 0.12.0 > > > The pipes code contains a few casts using the {{PByteArray}} pointer type. > The underlying array {{TByteArray}} is defined as {code} > type TByteArray = array [0..32767] of Byte; > {code} > With range checks enabled, any access to indices > 32767, even correct ones, > result in an range check exception. It may be worth noting that the code is > otherwise entirely correct (i.e. no buffer overruns). It is really only the > type cast that introduces the failing constraint. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 Hmm yeah must be a race condition with the test server starting in the go routine. Any thoughts on how to wait for it to start up? I guess I could use a wait group to wait at least until goroutine code begins to execute. Or I could have some retry logic that tries to contact the server for a second before failing ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350515#comment-16350515 ] ASF GitHub Bot commented on THRIFT-4448: Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 Hmm yeah must be a race condition with the test server starting in the go routine. Any thoughts on how to wait for it to start up? I guess I could use a wait group to wait at least until goroutine code begins to execute. Or I could have some retry logic that tries to contact the server for a second before failing > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 I think the only one-liner fix is a 500ms sleep before `client.TestVoid()`. Retry logic would work as well. ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350542#comment-16350542 ] ASF GitHub Bot commented on THRIFT-4448: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 I think the only one-liner fix is a 500ms sleep before `client.TestVoid()`. Retry logic would work as well. > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (THRIFT-4485) Possible invalid ptr AV with overlapped read/write on pipes
[ https://issues.apache.org/jira/browse/THRIFT-4485?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer resolved THRIFT-4485. Resolution: Fixed Fix Version/s: 0.12.0 Committed. > Possible invalid ptr AV with overlapped read/write on pipes > --- > > Key: THRIFT-4485 > URL: https://issues.apache.org/jira/browse/THRIFT-4485 > Project: Thrift > Issue Type: Bug > Components: Delphi - Library >Reporter: Jens Geyer >Assignee: Jens Geyer >Priority: Critical > Fix For: 0.12.0 > > > If a read or write operation on pipes reaches the set timeout, the read/write > operation is not properly cancelled. However, the overlapped struct gets > freed when leaving the method, which essentially leaves the pending read or > write operation with an undefined pointer. > Easily reproducible with buffered transport over pipes, a combination that > does not work at all anyways. The workaround for both problems is to not use > buffered transport with pipes (use framed instead), and some sane tinemouts > (not too short). -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (THRIFT-4454) Large writes/reads may cause range check errors in debug mode
[ https://issues.apache.org/jira/browse/THRIFT-4454?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer resolved THRIFT-4454. Resolution: Fixed Committed. > Large writes/reads may cause range check errors in debug mode > - > > Key: THRIFT-4454 > URL: https://issues.apache.org/jira/browse/THRIFT-4454 > Project: Thrift > Issue Type: Bug > Components: Delphi - Library >Affects Versions: 0.11.0 >Reporter: Jens Geyer >Assignee: Jens Geyer >Priority: Major > Fix For: 0.12.0 > > > The pipes code contains a few casts using the {{PByteArray}} pointer type. > The underlying array {{TByteArray}} is defined as {code} > type TByteArray = array [0..32767] of Byte; > {code} > With range checks enabled, any access to indices > 32767, even correct ones, > result in an range check exception. It may be worth noting that the code is > otherwise entirely correct (i.e. no buffer overruns). It is really only the > type cast that introduces the failing constraint. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (THRIFT-760) Generated client code does not set or check the sequence ID in messages
[ https://issues.apache.org/jira/browse/THRIFT-760?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350779#comment-16350779 ] Jens Geyer commented on THRIFT-760: --- http://thrift.apache.org/docs/HowToContribute > Generated client code does not set or check the sequence ID in messages > --- > > Key: THRIFT-760 > URL: https://issues.apache.org/jira/browse/THRIFT-760 > Project: Thrift > Issue Type: Bug > Components: Java - Compiler >Affects Versions: 0.2 >Reporter: James Grant >Assignee: James Grant >Priority: Minor > Fix For: 0.3 > > Attachments: ASF.LICENSE.NOT.GRANTED--thrift-seqid.patch > > > The sequence ID is never set in the generated client code. It is also never > checked. This means that if you continue to use a connection after a socket > timeout the returned results can arrive out of sequence. When this happens an > exception should be thrown so that incorrect results are never returned. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 It's not clear to me what went wrong in the Travis build. Trusty, for example, seems to have stack overflow'd while installing ocaml. Is there a way to retrigger a build without pushing another commit? ``` [default] synchronized from https://opam.ocaml.org Stack overflow [ERROR] Initialisation failed Fatal error: Stack overflow The command '/bin/sh -c apt-get install -y --no-install-recommends `# OCaml dependencies` ocaml opam && opam init --yes && opam install --yes oasis' returned a non-zero code: 1 ``` ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350795#comment-16350795 ] ASF GitHub Bot commented on THRIFT-4448: Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 It's not clear to me what went wrong in the Travis build. Trusty, for example, seems to have stack overflow'd while installing ocaml. Is there a way to retrigger a build without pushing another commit? ``` [default] synchronized from https://opam.ocaml.org Stack overflow [ERROR] Initialisation failed Fatal error: Stack overflow The command '/bin/sh -c apt-get install -y --no-install-recommends `# OCaml dependencies` ocaml opam && opam init --yes && opam install --yes oasis' returned a non-zero code: 1 ``` > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 The only other way is for someone with Travis crendentials to manually trigger it. I think it's faster if you push something trivial (whitespace etc.) Thrift's CI builds are unfortunately very hit and miss :/ ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350815#comment-16350815 ] ASF GitHub Bot commented on THRIFT-4448: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 The only other way is for someone with Travis crendentials to manually trigger it. I think it's faster if you push something trivial (whitespace etc.) Thrift's CI builds are unfortunately very hit and miss :/ > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 It bugged me to have a time.Sleep in the test so I wrote the retry logic :) ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350881#comment-16350881 ] ASF GitHub Bot commented on THRIFT-4448: Github user johnboiles commented on the issue: https://github.com/apache/thrift/pull/1459 It bugged me to have a time.Sleep in the test so I wrote the retry logic :) > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] thrift issue #1459: THRIFT-4448: Golang: do something with context.Context
Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 Looks *much* cleaner! ---
[jira] [Commented] (THRIFT-4448) Golang: do something with context.Context
[ https://issues.apache.org/jira/browse/THRIFT-4448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16350897#comment-16350897 ] ASF GitHub Bot commented on THRIFT-4448: Github user dcelasun commented on the issue: https://github.com/apache/thrift/pull/1459 Looks *much* cleaner! > Golang: do something with context.Context > - > > Key: THRIFT-4448 > URL: https://issues.apache.org/jira/browse/THRIFT-4448 > Project: Thrift > Issue Type: Task > Components: Go - Library >Affects Versions: 0.11.0 >Reporter: John Boiles >Priority: Major > > PR Here: https://github.com/apache/thrift/pull/1459 > This patch wires through {{context.Context}} such that it can be used in in > {{http.Request}}'s {{WithContext}} method. This allows Thrift HTTP requests > to canceled or timed out via the context. > This patch breaks support for go<1.7 so it's not ready to ship, but I'm > hoping to get some direction on this. When does Thrift expect to drop support > of go1.7? It looks like the current solution is to duplicate files that need > to use {{golang.org/x/net/context}} and add a {{// +build !go1.7}} but > duplication seems unsustainable as the {{context}} package is imported more > places. > Go 1.7 was released 15 August 2016. Given Golang has had significant > performance improvements in most dot releases, I suspect most production > services stay reasonably up to date. Here at Periscope/Twitter we're on > go1.9.1, and we're a fairly large organization. -- This message was sent by Atlassian JIRA (v7.6.3#76005)