#814 closed enhancement (invalid)
sem_setup should use SEM_UNDO flag
| Reported by: | Darac | Owned by: | nobody |
|---|---|---|---|
| Priority: | low | Milestone: | |
| Component: | master | Version: | 1.4.2 |
| Severity: | normal | Keywords: | |
| Cc: |
Description
The sem_setup function (in munin-cgi-graph and munin-fastcgi-graph) should use the SEM_UNDO flag in the call to semop to avoid overflowing the semaphore value.
man 2 semop states that the behaviour of semop() with a positive sem_op is to increment the semaphore value by this amount. The sem_*() functions use the value in the semaphore as a 'number of available jobs', decrementing the count before calling munin-graph and incrementing it afterwards. If sem_setup is called several times (as would happen with munin-cgi-graph), then the semaphore value is incremented by $max_cgi_graph_jobs each time. If the semaphore value would exceed SEMVMX, an error is returned and perl dies.
If we add SEM_UNDO, though, the amount we added to the semaphore in sem_setup is automatically removed when the process finishes.
The attached patch fixes this for 1.4.2
Attachments (1)
Change History (4)
Changed 3 years ago by Darac
comment:1 Changed 3 years ago by janl
- Resolution set to fixed
- Status changed from new to closed
munin-cgi-graph does not do a exec only, the preceding open is a implicit fork so the code after the exec is executed in the parent. But the undo is handy any way to make sure the process is cleaned up after it dies. Thanks. In r3251.
comment:2 Changed 3 years ago by janl
- Component changed from web-interface to master
- Milestone Munin 1.5 deleted
- Resolution fixed deleted
- Status changed from closed to reopened
The patch is wrong. Whatever you're trying to fix is broken some other way.
According to google SEM_UNDO is not very portable.
But most of all: We MUST NOT undo the semaphore initialization, it would leave the semaphore with a 0 value at process exit which is not what is needed.
Reverted the patch. If you still think it is correct please reopen the ticket and provide a better explanation.
comment:3 Changed 3 years ago by janl
- Resolution set to invalid
- Status changed from reopened to closed

Add SEM_UNDO flag (0x1000) to the semop call