diff options
| -rw-r--r-- | courses.go | 53 | ||||
| -rw-r--r-- | err.go | 1 | ||||
| -rw-r--r-- | index.go | 23 | ||||
| -rw-r--r-- | wsc.go | 43 | ||||
| -rw-r--r-- | wsm.go | 53 | ||||
| -rw-r--r-- | wsp.go | 15 |
6 files changed, 115 insertions, 73 deletions
@@ -94,24 +94,9 @@ func checkCourseGroup(cg courseGroupT) bool { return courseGroups[cg] } -/* - * The courses are simply stored in a map indexed by the course ID, although - * the course struct itself also contains an ID field. A lock is embedded - * inside the struct; we use a lock here instead of a pointer to a lock as - * it would be easy to forget to initialize the lock when creating the - * struct. However, this means that the struct could not be copied (though - * this should only ever happen during creation anyways), therefore we use a - * pointer to the struct as the value of the map, instead of the struct itself. - */ -var courses map[int](*courseT) +var courses sync.Map /* int, *courseT */ -/* - * This RWMutex is only for massive modifications of the course struct, since - * locking it on every write would be inefficient; in normal operation the only - * write that could occur to the courses struct is changing the Selected - * number, which should be handled with courseT.SelectedLock. - */ -var coursesLock sync.RWMutex +var numCourses uint32 /* * Read course information from the database. This should be called during @@ -119,11 +104,6 @@ var coursesLock sync.RWMutex * a null pointer dereference. */ func setupCourses() error { - coursesLock.Lock() - defer coursesLock.Unlock() - - courses = make(map[int](*courseT)) - rows, err := db.Query( context.Background(), "SELECT id, nmax, title, ctype, cgroup, teacher, location FROM courses", @@ -184,7 +164,8 @@ func setupCourses() error { err, ) } - courses[currentCourse.ID] = ¤tCourse + courses.Store(currentCourse.ID, ¤tCourse) + atomic.AddUint32(&numCourses, 1) } return nil @@ -231,11 +212,19 @@ func populateUserCourseGroups( ) } var thisGroupName courseGroupT - func() { - coursesLock.RLock() - defer coursesLock.RUnlock() - thisGroupName = courses[thisCourseID].Group - }() + _course, ok := courses.Load(thisCourseID) + if !ok { + return fmt.Errorf( + "%w: %d", + errNoSuchCourse, + thisCourseID, + ) + } + course, ok := _course.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } + thisGroupName = course.Group if _, ok := (*userCourseGroups)[thisGroupName]; ok { return fmt.Errorf( "%w: user %v, group %v", @@ -258,7 +247,7 @@ func (course *courseT) decrementSelectedAndPropagate( defer course.SelectedLock.Unlock() atomic.AddUint32(&course.Selected, ^uint32(0)) }() - go propagateSelectedUpdate(course.ID) + go propagateSelectedUpdate(course) err := sendSelectedUpdate(ctx, conn, course.ID) if err != nil { return fmt.Errorf( @@ -269,9 +258,3 @@ func (course *courseT) decrementSelectedAndPropagate( } return nil } - -func getCourseByID(courseID int) *courseT { - coursesLock.RLock() - defer coursesLock.RUnlock() - return courses[courseID] -} @@ -44,4 +44,5 @@ var ( errCannotGenerateRandomString = errors.New("cannot generate random string") errContextCancelled = errors.New("context cancelled") errCannotReceiveMessage = errors.New("cannot receive message") + errNoSuchCourse = errors.New("no such course") ) @@ -109,9 +109,26 @@ func handleIndex(w http.ResponseWriter, req *http.Request) { return } + /* + * Copy courses to _courses. The former is a sync.Map and the latter is + * a map[int]*courseT, and the former is very difficult to access from + * HTML templates. + */ + _courses := make(map[int]*courseT) + courses.Range(func(key, value interface{}) bool { + courseID, ok := key.(int) + if !ok { + panic("courses map has non-\"int\" keys") + } + course, ok := value.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } + _courses[courseID] = course + return true + }) + err = func() error { - coursesLock.RLock() - defer coursesLock.RUnlock() return tmpl.ExecuteTemplate( w, "student", @@ -121,7 +138,7 @@ func handleIndex(w http.ResponseWriter, req *http.Request) { "Name": userName, "Department": userDepartment, }, - "courses": courses, + "courses": &_courses, }, ) }() @@ -86,24 +86,37 @@ func handleConn( /* TODO: Tell the user their current choices here. Deprecate HELLO. */ usems := make(map[int]*usemT) - func() { - atomic.AddInt64(&usemCount, int64(len(courses))) - coursesLock.RLock() - defer coursesLock.RUnlock() - for courseID, course := range courses { - usem := &usemT{} //exhaustruct:ignore - usem.init() - course.Usems.Store(userID, usem) - usems[courseID] = usem + + /* TODO: Check if the LoadUint32 here is a bit too much overhead */ + atomic.AddInt64(&usemCount, int64(atomic.LoadUint32(&numCourses))) + courses.Range(func(key, value interface{}) bool { + /* TODO: Remember to change this too when changing the courseID type */ + courseID, ok := key.(int) + if !ok { + panic("courses map has non-\"int\" keys") } - }() + course, ok := value.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } + usem := &usemT{} //exhaustruct:ignore + usem.init() + course.Usems.Store(userID, usem) + usems[courseID] = usem + return true + }) + defer func() { - coursesLock.RLock() - defer coursesLock.RUnlock() - for _, course := range courses { + courses.Range(func(key, value interface{}) bool { + _ = key + course, ok := value.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } course.Usems.Delete(userID) - } - atomic.AddInt64(&usemCount, -int64(len(courses))) + return true + }) + atomic.AddInt64(&usemCount, -int64(atomic.LoadUint32(&numCourses))) }() usemParent := make(chan int) @@ -104,15 +104,20 @@ func messageChooseCourse( return reportError("Course ID must be an integer") } courseID := int(_courseID) - course := getCourseByID(courseID) + _course, ok := courses.Load(courseID) + if !ok { + return reportError("no such course") + } + course, ok := _course.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } if course == nil { - return reportError("nil course") + return reportError("couse is nil") } - thisCourseGroup := course.Group - - if _, ok := (*userCourseGroups)[thisCourseGroup]; ok { + if _, ok := (*userCourseGroups)[course.Group]; ok { err := writeText(ctx, c, "R "+mar[1]+" :Group conflict") if err != nil { return fmt.Errorf( @@ -181,7 +186,7 @@ func messageChooseCourse( }() if ok { - go propagateSelectedUpdate(courseID) + go propagateSelectedUpdate(course) err := tx.Commit(ctx) if err != nil { err := course.decrementSelectedAndPropagate(ctx, c) @@ -201,7 +206,7 @@ func messageChooseCourse( * This would race if message handlers could run * concurrently for one connection. */ - (*userCourseGroups)[thisCourseGroup] = struct{}{} + (*userCourseGroups)[course.Group] = struct{}{} err = writeText(ctx, c, "Y "+mar[1]) if err != nil { @@ -275,10 +280,17 @@ func messageUnchooseCourse( return reportError("Course ID must be an integer") } courseID := int(_courseID) - course := getCourseByID(courseID) + _course, ok := courses.Load(courseID) + if !ok { + return reportError("no such course") + } + course, ok := _course.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } if course == nil { - return reportError("nil course") + return reportError("couse is nil") } ct, err := db.Exec( @@ -302,16 +314,23 @@ func messageUnchooseCourse( err, ) } - var thisCourseGroup courseGroupT - func() { - coursesLock.RLock() - defer coursesLock.RUnlock() - thisCourseGroup = courses[courseID].Group - }() - if _, ok := (*userCourseGroups)[thisCourseGroup]; !ok { + + _course, ok := courses.Load(courseID) + if !ok { + return reportError("no such course") + } + course, ok := _course.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } + if course == nil { + return reportError("couse is nil") + } + + if _, ok := (*userCourseGroups)[course.Group]; !ok { return reportError("inconsistent user course groups") } - delete(*userCourseGroups, thisCourseGroup) + delete(*userCourseGroups, course.Group) } err = writeText(ctx, c, "N "+mar[1]) @@ -80,8 +80,7 @@ func makeReportError(ctx context.Context, conn *websocket.Conn) reportErrorT { } } -func propagateSelectedUpdate(courseID int) { - course := courses[courseID] +func propagateSelectedUpdate(course *courseT) { course.Usems.Range(func(key, value interface{}) bool { _ = key usem, ok := value.(*usemT) @@ -98,7 +97,17 @@ func sendSelectedUpdate( conn *websocket.Conn, courseID int, ) error { - course := courses[courseID] + _course, ok := courses.Load(courseID) + if !ok { + return fmt.Errorf("%w: %d", errNoSuchCourse, courseID) + } + course, ok := _course.(*courseT) + if !ok { + panic("courses map has non-\"*courseT\" items") + } + if course == nil { + return fmt.Errorf("%w: %d", errNoSuchCourse, courseID) + } selected := atomic.LoadUint32(&course.Selected) err := writeText(ctx, conn, fmt.Sprintf("M %d %d", courseID, selected)) if err != nil { |