Permalink
Browse files

bug24329: avoid races in constructor initialization

%Testing: make stress-aserve

Avoid calls to make-instance when the class is defined later in the file
or in a file loaded later.  Add make-instance-foo functions for these
classes.

Add a separate make-instance-foo function for each set of keyword arguments
desired by the caller, to allow the make-instance call to be optimized.

When a compiled function is loaded containing a make-instance call for a
class that is not yet loaded, a load-form creates a constructor object
that needs to be initialized the first time it is used.  Since the
initialization involves modifying several places in memory, it must
not be done simultaneously in several threads.

Change-Id: I60663fb63d4b4fa050607f9bac6bfb2bfc90491b
Reviewed-on: https://gerrit.franz.com:9080/6737
Reviewed-by: Martin Mikelsons <mm@franz.com>
Reviewed-by: Duane Rettig <duane@franz.com>
Reviewed-by: Kevin Layer <layer@franz.com>
Reviewed-by: Robert Rorschach <rfr@franz.com>
Tested-by: Kevin Layer <layer@franz.com>
  • Loading branch information...
1 parent 2803a5b commit 0cb53c5cf0e1f5b0eabefdd1830d8eb9c9841dbb Martin Mikelsons committed with dklayer Nov 14, 2016
Showing with 72 additions and 42 deletions.
  1. +9 −0 authorize.cl
  2. +10 −0 chunker.cl
  3. +1 −2 client.cl
  4. +17 −12 main.cl
  5. +3 −2 packages.cl
  6. +18 −10 publish.cl
  7. +1 −3 webactions/clpage.cl
  8. +4 −12 webactions/webact.cl
  9. +9 −1 webactions/websession.cl
View
@@ -36,6 +36,10 @@
:initform "AllegroServe")
))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-password-authorizer+realm+allowed (realm allowed)
+ (make-instance 'password-authorizer :realm realm :allowed allowed))
+
(defmethod authorize ((auth password-authorizer)
@@ -112,6 +116,11 @@
:initarg :patterns
:initform nil)))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-location-authorizer+patterns (patterns)
+ (make-instance 'location-authorizer :patterns patterns))
+
+
View
@@ -20,6 +20,10 @@
((trailers :initform nil :accessor chunking-stream-trailers)
(eof-sent :initform nil :accessor chunking-stream-eof-sent)))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-chunking-stream+output-handle (output-handle)
+ (make-instance 'chunking-stream :output-handle output-handle))
+
(defmethod chunking-stream-trailers (stream)
;; so this will return nil for non chunkers
@@ -232,6 +236,12 @@
:accessor unchunking-trailers)
))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-unchunking-stream+input-handle (input-handle)
+ (make-instance 'unchunking-stream :input-handle input-handle))
+
+
+
(defmethod unchunking-trailers ((stream inflate-stream))
(unchunking-trailers (slot-value stream 'excl::input-handle)))
View
@@ -226,8 +226,7 @@
; the data is coming in chunked then unchunk it
(if* (eq :chunked left)
then (setq socket
- (make-instance 'net.aserve::unchunking-stream
- :input-handle socket)))
+ (net.aserve::make-instance-unchunking-stream+input-handle socket)))
; if data is content-encoded, then decode
(if* (client-request-content-encoding creq)
View
@@ -19,7 +19,7 @@
#+ignore
(check-smp-consistency)
-(defparameter *aserve-version* '(1 3 44))
+(defparameter *aserve-version* '(1 3 45))
(eval-when (eval load)
(require :sock)
@@ -472,10 +472,8 @@ died. Use nil to specify no timeout.")
(locators
;; list of locators objects in search order
- :initform (list (make-instance 'locator-exact
- :name :exact)
- (make-instance 'locator-prefix
- :name :prefix))
+ :initform (list (make-instance-locator-exact+name :exact)
+ (make-instance-locator-prefix+name :prefix))
:accessor wserver-locators)
(filters
@@ -529,7 +527,7 @@ died. Use nil to specify no timeout.")
(default-vhost
;; vhost representing situations with no virtual host
:initarg :default-vhost
- :initform (make-instance 'vhost)
+ :initform (make-instance-vhost-0)
:accessor wserver-default-vhost)
(response-timeout
@@ -697,6 +695,10 @@ died. Use nil to specify no timeout.")
:initform nil)
))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-vhost-0 ()
+ (make-instance 'vhost))
+
(defmethod print-object ((vhost vhost) stream)
(print-unreadable-object (vhost stream :type t :identity t)
(format stream "~{ ~a~}"
@@ -2156,9 +2158,7 @@ by keyword symbols and not by strings"
:element-type 'character
:adjustable t
:fill-pointer 0))
- (sock (make-instance
- 'unchunking-stream
- :input-handle (request-socket req)))
+ (sock (make-instance-unchunking-stream+input-handle (request-socket req)))
(ch))
(loop (if* (eq :eof
(setq ch (read-char sock nil :eof)))
@@ -3489,10 +3489,9 @@ in get-multipart-sequence"))
(values nil nil)
(let* ((length (parse-integer (header-slot-value req :content-length) :junk-allowed t))
(str (cond (length
- (make-instance 'truncated-stream :byte-length length
- :input-handle (request-socket req)))
+ (make-instance-truncated-stream+byte-length+input-handle length (request-socket req)))
((equalp "chunked" (header-slot-value req :transfer-encoding))
- (make-instance 'unchunking-stream :input-handle (request-socket req)))
+ (make-instance-unchunking-stream+input-handle (request-socket req)))
(t (request-socket req))))
(enc (header-slot-value req :content-encoding))
entry must-close (must-flush t))
@@ -3521,6 +3520,12 @@ in get-multipart-sequence"))
(def-stream-class truncated-stream (terminal-simple-stream)
((remaining :initarg :byte-length :accessor octets-remaining)))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-truncated-stream+byte-length+input-handle
+ (byte-length input-handle)
+ (make-instance 'truncated-stream :byte-length byte-length :input-handle input-handle))
+
+
(defmethod print-object ((obj truncated-stream) stream)
(print-unreadable-object (obj stream :type t :identity t)
(format stream "~:d remaining" (octets-remaining obj))))
View
@@ -5,8 +5,9 @@
;; See the file LICENSE for the full license governing this code.
#+(version= 10 0)
-(sys:defpatch "aserve" 10
- "v10: no version change, fix defpatch;
+(sys:defpatch "aserve" 11
+ "v11: 1.3.45 - avoid races in constructor initialization;
+v10: no version change, fix defpatch;
v9: 1.3.44: add :test-ssl argument to start function;
v8: 1.3.43: don't log when client closes connection early;
v7: 1.3.42: internal improvements to server body access;
View
@@ -252,12 +252,20 @@
()
(:default-initargs :info (make-hash-table :test #'equal)))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-locator-exact+name (name)
+ (make-instance 'locator-exact :name name))
+
(defclass locator-prefix (locator)
;; use to map prefixes to entities
()
)
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-locator-prefix+name (name)
+ (make-instance 'locator-prefix :name name))
+
;; the info slot of a locator-prefix class is a list of
;; prefix-handler objects, sorted by the length of the path
@@ -1918,16 +1926,14 @@
(let ((ip (assoc :ip info :test #'eq)))
(if* ip
then (setq ip-authorizer
- (make-instance 'location-authorizer
- :patterns (getf (cdr ip) :patterns)))))
+ (make-instance-location-authorizer+patterns (getf (cdr ip) :patterns)))))
; only one of ip and pswd allowed
(let ((pswd (assoc :password info :test #'eq)))
(if* pswd
then (setq pswd-authorizer
- (make-instance 'password-authorizer
- :realm (getf (cdr pswd) :realm)
- :allowed (getf (cdr pswd) :allowed)))))
+ (make-instance-password-authorizer+realm+allowed
+ (getf (cdr pswd) :realm) (getf (cdr pswd) :allowed)))))
; check password second
(if* pswd-authorizer
@@ -2707,9 +2713,8 @@
(setq reply-stream
(setf (request-reply-stream req)
- (make-instance 'prepend-stream
- :content header-content
- :output-handle reply-stream)))))
+ (make-instance-prepend-stream+content+output-handle
+ header-content reply-stream)))))
@@ -2718,8 +2723,7 @@
then (force-output hsock)
; do chunking
(setq reply-stream
- (make-instance 'chunking-stream
- :output-handle reply-stream))
+ (make-instance-chunking-stream+output-handle reply-stream))
(setf (request-reply-stream req)
reply-stream)
(setf (chunking-stream-trailers reply-stream)
@@ -2925,6 +2929,10 @@
)
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-prepend-stream+content+output-handle (content output-handle)
+ (make-instance 'prepend-stream :content content :output-handle output-handle))
+
(defmethod prepend-stream-p (stream)
(declare (ignore stream))
@@ -167,9 +167,7 @@ v1: add timeout to webaction-project."
then (setf (websession-method websession)
:cookie))
else ; no session found via cookie
- (setq websession (make-instance 'websession
- :key (next-websession-id sm)
- :method :cookie))
+ (setq websession (make-instance-websession+key+method (next-websession-id sm) :cookie))
(setf (gethash (websession-key websession)
(sm-websessions sm))
websession)))
View
@@ -168,9 +168,7 @@
sessions)
then (initialize-websession-master
(setf (webaction-websession-master wa)
- (make-instance 'websession-master
- :cookie-name name
- :reap-hook-function reap-hook-function
+ (make-instance-websession-master+cookie-name+reap-hook-function name reap-hook-function
))))
(if* (null sessions)
@@ -295,9 +293,7 @@
then ; no session found, but this session id looks good
; and was probably created by another web server.
; so create a session object
- (setq websession (make-instance 'websession
- :key csessid
- :method :cookie))
+ (setq websession (make-instance-websession+key+method csessid :cookie))
(setf (gethash csessid (sm-websessions sm)) websession)))
(if* websession
then (setf (websession-from-req req) websession)))
@@ -336,9 +332,7 @@
(setf (websession-from-req req)
(setf (gethash key (sm-websessions sm))
- (setq websession (make-instance 'websession
- :key key
- :method :try-cookie))))))
+ (setq websession (make-instance-websession+key+method key :try-cookie))))))
@@ -571,9 +565,7 @@ no map for webaction with default-actions ~s"
then ; add new session
(setf (websession-from-req req)
(setf (gethash sessid (sm-websessions sm))
- (make-instance 'websession
- :key sessid
- :method :try-cookie)))
+ (make-instance-websession+key+method sessid :try-cookie)))
))
@@ -41,7 +41,12 @@
:reader sm-cookie-name)
(websessions :initform (make-hash-table :test #'equal)
- :reader sm-websessions)))
+ :reader sm-websessions)))
+
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-websession-master+cookie-name+reap-hook-function (name reap-hook-function)
+ (make-instance 'websession-master :name name :reap-hook-function reap-hook-function))
+
(defclass websession ()
@@ -70,6 +75,9 @@
(variables :initform nil
:accessor websession-variables)))
+;; Mention class in make-instance after class def to avoid bug24329.
+(defun make-instance-websession+key+method (key method)
+ (make-instance 'websession :key key :method method))

0 comments on commit 0cb53c5

Please sign in to comment.