Opened 6 years ago

Closed 4 years ago

#1041 closed defect (fixed)

-d0 causes different application behavior

Reported by: Mario Domenech Goulart Owned by:
Priority: critical Milestone: 4.11.0
Component: unknown Version: 4.8.x
Keywords: Cc: andyjpb@…
Estimated difficulty:

Description

When compiled with -d0, awful-picman fails to serve some files. It works fine when compiled with -d1.

I'll try to provide a shorter test case, but for now the steps to reproduce are:

  1. Clone the awful-picman repo (https://github.com/mario-goulart/awful-picman)
  1. cd awful-picman
  1. git checkout e810ddc78da725051eb76e9b205358a5c23b7912
  1. chicken-install
  1. mkdir foo
  1. cd foo
  1. awful-picman --init &
  1. curl http://localhost:8080/css/awful-picman.css
    <?xml version="1.0" encoding="utf-8" ?>
    <!DOCTYPE html
      PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
             "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml"
          xml:lang="en" lang="en">
      <head>
        <title>404 - Not Found</title>
      </head>
      <body>
        <h1>404 - Not Found</h1>
        <p>The resource you requested could not be found</p>
      </body>
    </html>
    

If I change the build option from -d0 to -d1 in the .setup file (line 11) and repeat steps from 4 to 8, it works:

$ curl http://localhost:8080/css/awful-picman.css | head
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1172  100  1172    0     0   368k      0 --:--:-- --:--:-- --:--:--  572k
body {
    padding-top: 80px;
}

.breadcrumb {
    top: 40px;
    position: fixed;
}

.dir {

Tested with:

$ chicken -version
(c) 2008-2013, The Chicken Team
(c) 2000-2007, Felix L. Winkelmann
Version 4.8.0.3 (stability/4.8.0) (rev 091c3d9)
linux-unix-gnu-x86-64 [ 64bit manyargs dload ptables ]
compiled 2013-03-12 on aeryn.xorinia.dim (Darwin)
$ gcc --version
gcc (Debian 4.7.2-5) 4.7.2
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Change History (20)

comment:1 Changed 6 years ago by Mario Domenech Goulart

It seems that -no-lambda-info is the option which actually triggers the problem (-d0 implies -no-lambda-info).

comment:2 Changed 6 years ago by Mario Domenech Goulart

Here's a shorter test case that can be used to reproduce the problem:

$ cat test.scm
(use awful spiffy)

;; just create a test file:
(with-output-to-file "file.txt" (cut print "file.txt"))

(define (app)

  (define (foo-matcher req-path)
    (and (equal? req-path "/foo-page")
	 (list req-path)))

  (define-page foo-matcher
    (lambda (req-path)
      req-path))

  (define (bar-matcher req-path)
    (and (equal? req-path "/bar-page")
	 (list req-path)))

  (define-page bar-matcher
    (lambda (req-path)
      (lambda ()
	(send-static-file "file.txt"))))

  ) ;; end app


(awful-start app dev-mode?: #t)

Expected behavior:

$ csc test.scm && ./test

$ curl http://localhost:8080/foo-page
<html><head></head><body>/foo-page</body></html>

Unexpected behavior (compiling with -no-lambda-info):

$ csc -no-lambda-info test.scm && ./test

$ curl http://localhost:8080/foo-page
file.txt

If I replace the handler for the bar-page from

(lambda (req-path)
  (lambda ()
    (send-static-file "file.txt")))

to

(lambda (req-path)
  req-path)

I get the expected result, i.e.,

curl http://localhost:8080/foo-page
<html><head></head><body>/foo-page</body></html>

comment:3 Changed 6 years ago by Mario Domenech Goulart

Resolution: fixed
Status: newclosed

It seems to be fixed in master. I'll run a git bisect to find out what fixed it.

comment:4 Changed 6 years ago by sjamaan

My bisect run seemed to indicate 799b4b27a557232e6455791bdfd532f950f0fbdb was the one that fixed the bug. Doesn't make much sense to me though, so maybe something went wrong in mixing up git bisect "good" and "bad" arguments.

comment:5 Changed 6 years ago by Mario Domenech Goulart

It seems that 799b4b27a557232e6455791bdfd532f950f0fbdb really fixes the issue. I get the expected behavior with it. I get the unexpected behavior with the commit immediately before it (56ee10352fdd029e0e9c434bf816defa270e8fd1). I just don't understand yet how it fixes it.

comment:6 Changed 6 years ago by Mario Domenech Goulart

Resolution: fixed
Status: closedreopened

I've found another case that triggers that problem, this time it is reproducible with the 4.8.2 dev-snapshot tarball (I cannot reproduce the problem using the 4.8.2 dev-snapshot tarball and the test case in comment:2).

Unfortunately, I couldn't come up with a simpler test case so far. What I have now is:

  • cd awful-path-matchers

The expected behavior:

$ chicken-install -test
retrieving ...
checking platform for 'awful-path-matchers' ...
checking dependencies for 'awful-path-matchers' ...
install order:
("awful-path-matchers")
installing awful-path-matchers: ...
changing current directory to .
  '/home/mario/local/chicken-4.8.2/bin/csi' -bnq -setup-mode -e "(require-library setup-api)" -e "(import setup-api)" -e "(setup-error-handling)" -e "(extension-name-and-version '(\"awful-path-matchers\" \"\"))" 'awful-path-matchers.setup'
  '/home/mario/local/chicken-4.8.2/bin/csc' -feature compiling-extension -setup-mode    -d1 -O3 -J -s awful-path-matchers.scm
  '/home/mario/local/chicken-4.8.2/bin/csc' -feature compiling-extension -setup-mode    -d1 -O3 -s awful-path-matchers.import.scm
  cp -r 'awful-path-matchers.so' '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.so'
  cp -r 'awful-path-matchers.import.so' '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.import.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.import.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.setup-info'
  '/home/mario/local/chicken-4.8.2/bin/csi' -s run.scm awful-path-matchers 

Starting test-server on port 8080
Standby...(get "/foo/2") ....................................................... [ PASS]
(get "/foo/2/3") ..................................................... [ PASS]
Shutting down
Sent SIGTERM to server. Please make sure the server isn't running anymore!

The unexpected behavior:

$ CSC_OPTIONS=-no-lambda-info chicken-install -test
retrieving ...
checking platform for 'awful-path-matchers' ...
checking dependencies for 'awful-path-matchers' ...
install order:
("awful-path-matchers")
installing awful-path-matchers: ...
changing current directory to .
  '/home/mario/local/chicken-4.8.2/bin/csi' -bnq -setup-mode -e "(require-library setup-api)" -e "(import setup-api)" -e "(setup-error-handling)" -e "(extension-name-and-version '(\"awful-path-matchers\" \"\"))" 'awful-path-matchers.setup'
  '/home/mario/local/chicken-4.8.2/bin/csc' -feature compiling-extension -setup-mode    -d1 -O3 -J -s awful-path-matchers.scm
  '/home/mario/local/chicken-4.8.2/bin/csc' -feature compiling-extension -setup-mode    -d1 -O3 -s awful-path-matchers.import.scm
  cp -r 'awful-path-matchers.so' '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.so'
  cp -r 'awful-path-matchers.import.so' '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.import.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.import.so'
  chmod a+r '/home/mario/local/chicken-4.8.2/lib/chicken/7/awful-path-matchers.setup-info'
  '/home/mario/local/chicken-4.8.2/bin/csi' -s run.scm awful-path-matchers 

Starting test-server on port 8080
Standby...(get "/foo/2") ....................................................... [ERROR]
    
Error: (call-with-input-request) Server error: 500 Internal server error: #<URI-common: scheme=http port=8080 host="localhost" path=(/ "foo" "2") query=()...#f>
(get "/foo/2/3") ..................................................... [ERROR]
    
Error: (call-with-input-request) Client error: 404 Not Found: #<URI-common: scheme=http port=8080 host="localhost" path=(/ "foo" "2" "3") quer...()...#f>
Shutting down
Sent SIGTERM to server. Please make sure the server isn't running anymore!


Error: shell command terminated with nonzero exit code
256
"'/home/mario/local/chicken-4.8.2/bin/csi' -s run.scm awful-path-matchers "

comment:7 Changed 6 years ago by andyjpb

cd /tmp; mkdir -p web/x; echo "hi" >> web/x/hello.txt; sudo chown -R root.root web/x; sudo chmod 755 web/x; sudo chmod 600 web/x/hello.txt; csi -e '(use spiffy) (start-server)';

In another shell:

curl -v http://localhost:8080/x/hello.txt

I get a 200 response and then the text "HTT". The server throws and exception.

I was expecting a 403.

In my original case (for which I was trying to make a simple repro) I was getting a 403 but the HTTP connection was getting wedged.

Spiffy 5, Chicken 4.7.0.

comment:8 Changed 6 years ago by sjamaan

The Spiffy problem mentioned by Andy turned out to be a boring old simple bug in Spiffy itself, so please disregard it with respect to this big bug in CHICKEN itself.

comment:9 in reply to:  2 Changed 6 years ago by andyjpb

Cc: andyjpb added

Replying to mario:

Here's a shorter test case that can be used to reproduce the problem:

Expected behavior:

$ csc test.scm && ./test

$ curl http://localhost:8080/foo-page
<html><head></head><body>/foo-page</body></html>

Unexpected behavior (compiling with -no-lambda-info):

$ csc -no-lambda-info test.scm && ./test

$ curl http://localhost:8080/foo-page
file.txt

I can reproduce this under 64-bit Linux with Chicken 4.7.0 and awful 0.39.2.

comment:10 Changed 6 years ago by andyjpb

Cc: andyjpb removed

comment:11 Changed 6 years ago by andyjpb

Cc: andyjpb@… added

comment:12 Changed 6 years ago by Mario Domenech Goulart

It seems that the issue is somehow related to hash tables. I modified awful to use alists instead of hash tables to store the resources table. I cannot reproduce the awful-path-matchers issue using the modified awful.

To install the modified awful:

$ git clone https://github.com/mario-goulart/awful.git
$ git checkout alist
$ chicken-install

To see the changes in awful which implement the resources table with an alist:

$ git diff 46a37fbe83b5dc327fd25b048747029f4e2ccf7b

With the modified awful installed, I get the expected behavior in awful-path-matchers' tests, no matter if I use -no-lambda-info or not.

comment:13 Changed 6 years ago by felix winkelmann

Yes, "-d0" modifies the allocation behavior, which in CHICKEN has a direct impact on execution (in terms of performance, and in the number and times where GC happens, and so). The use of lambda-info allocates more storage for closure records and thus influences the nursery-allocation frequency.

This is an inherent factor that should always be kept in mind. The bug reported here is somewhere else - "-d0" just modifies the pattern of memory-access.

comment:14 Changed 6 years ago by sjamaan

Milestone: 4.9.04.10.0

This is so weird we won't be able to pinpoint it for 4.9.0 (unless someone is interested in doing a heroic last-minute crazy hacking sprint)

comment:15 Changed 4 years ago by sjamaan

Milestone: 4.10.04.11.0

Moving it once again....

comment:16 Changed 4 years ago by sjamaan

Looking at it, it seems like awful always picks the last defined page rather than the one which matches the path...

comment:17 Changed 4 years ago by sjamaan

OK, here we go:

#;1> (use awful-path-matchers srfi-69)
#;2> (##sys#peek-fixnum (path-match "foo" <int>) 0)
140289424854051
#;3> (##sys#size (path-match "foo" <int>))
3
#;4> (##sys#slot (path-match "foo" <int>) 0)
70144712427025
#;5> (##sys#slot (path-match "foo" <int>) 1)
("foo" #<procedure (awful-path-matchers#<int> thing17)>)
#;6> (##sys#slot (path-match "foo" <int>) 2)
#<lambda info (f_525 path90)>
#;7> (define equiv? (hash-table-equivalence-function (make-hash-table)))
#;8> (equiv? (path-match "foo" <int> <int>) (path-match "foo" <int>))
#f

Without lambda info:

#;1> (use awful-path-matchers srfi-69)
#;2> (##sys#peek-fixnum (path-match "foo" <int>) 0)
140155045890064
#;3> (##sys#size (path-match "foo" <int>))
2
#;4> (##sys#slot (path-match "foo" <int>) 0)
70178053519952
#;5> (##sys#slot (path-match "foo" <int>) 1)
("foo" #<procedure>)
#;6> (##sys#size (cadr #5))
1
#;7> (define equiv? (hash-table-equivalence-function (make-hash-table)))
#;8> (equiv? (path-match "foo" <int> <int>) (path-match "foo" <int>))
#t

I think the problem here is the fact that, for example, (path-match "foo" <int>) creates a closure containing a rest list with the string "foo" and a procedure which is not inspectable. If then you do (path-match "foo" <string>), it also creates a closure containing a rest list with the string "foo" and a procedure which is not inspectable.

If you look at the way the srfi-69 hashing works, it will hash "special" objects (closures are such) by looking at all the slots except the first (because it doesn't contain a Scheme object). Without lambda info, the closure objects have one less slot.

See %%special-vector-hash. The very strange thing is that it uses ##sys#peek-fixnum to extract the procedure's memory location from the closure and convert it to a fixnum, which it then uses as a random seed!

And all of that aside, like I said the lists are NOT equivalent so that should not make a difference, I think... Tweaking recursive-hash-max-length or recursive-hash-max-depth doesn't seem to help.

comment:18 Changed 4 years ago by sjamaan

D'oh! Actually, hash tables seem to have nothing to do with it!

#;1> (use srfi-69 awful-path-matchers)
#;2> (define equiv? (hash-table-equivalence-function (make-hash-table)))
#;3> equiv?
#<procedure (equal? x321 y322)>
#;4> (equiv? (path-match "foo" <int> <int>) (path-match "foo" <int>))
#t
#;5> (equal? (path-match "foo" <int> <int>) (path-match "foo" <int>))
#t

comment:19 Changed 4 years ago by sjamaan

(define (constantly x) (lambda () x))
(print (equal? (constantly (string)) (constantly (string))))

If you compile this with -no-lambda-info, it prints #t. If you compile it without this option, it'll print #f. This is due an off-by-one where comparison would skip the final slot in the closure object and needlessly compare the headers. Patch sent to chicken-hackers.

comment:20 Changed 4 years ago by sjamaan

Resolution: fixed
Status: reopenedclosed

The underlying problem has now been fixed with eb15ee6 / 3545d5e

Note: See TracTickets for help on using tickets.